Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:118836 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 7621 invoked from network); 18 Oct 2022 01:27:09 -0000 Received: from unknown (HELO php-smtp4.php.net) (45.112.84.5) by pb1.pair.com with SMTP; 18 Oct 2022 01:27:09 -0000 Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 5809C180044 for ; Mon, 17 Oct 2022 18:27:08 -0700 (PDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,BODY_8BITS, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.2 X-Spam-ASN: AS15169 209.85.128.0/17 X-Spam-Virus: No X-Envelope-From: Received: from mail-pj1-f46.google.com (mail-pj1-f46.google.com [209.85.216.46]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Mon, 17 Oct 2022 18:27:07 -0700 (PDT) Received: by mail-pj1-f46.google.com with SMTP id pq16so12591364pjb.2 for ; Mon, 17 Oct 2022 18:27:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=wikimedia.org; s=google; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=4mrAsHz6w8jwGIdETSRPW5yNRUv+W6S1RKr6W6EzK4E=; b=petRs+4QuHcbUqVDhjA2b0QZJBZIjIfvCOyvMyaMGgYQoMBU9n7A8OP/dBIacPALfA AvYfLdrMzw1QSuM7s8GmBQf6Pld5cIx1hfekFQNmlytA6BjBjxsouavY9++jXH/GTCDU +2TyMe6SPwwQ/33O1tGce/YPHEFPN8lpxKdQw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=4mrAsHz6w8jwGIdETSRPW5yNRUv+W6S1RKr6W6EzK4E=; b=jAB/i5Q2PyXFIXnpqRbGf6oh9Qp6kVZSStU0D9YoSeBpVE0zDQdfQ950UKe+wm7hV5 jBwb5WZbTgRUUAItMfraOquVwcyBIKYhjLWW7ZCJCwqPOOcTh3wNOP+9Gio7OyRQKLIT 2dtfNBPot4olvfMrJ4hrHFQnGs2Nqfzo6bZvqCM9MHztM6u+lJqxTIZIzfT6tPxb0Wn3 TsQVo91u4XerjOFB5Hp3P8hkrplazetiBZn1GX7bWd6OyS37dwbTn9Qsvzk1nItgK4Sf GbhOCyN5ZLp97isDErJLpJsTAhA2w5Rf9fY7Md5X5MYZ5y7nfD4RiUrIxalwkifDdO5C KCYA== X-Gm-Message-State: ACrzQf2b96lO3uoyh6DPVDZ9wSyxtQNNZQAekdsitZfeN/ThqzftYikZ p4e3lio627UFSObdT5Ll+KkMeUWtKLK1lg== X-Google-Smtp-Source: AMsMyM4YFWf6HyFgOxEoJESuct746Evr+QSTuu9zsrcYnp+/Z4Gzm1F9VJXlMBy765xoI/pm1hEsYg== X-Received: by 2002:a17:90a:f192:b0:20d:2ea6:4b18 with SMTP id bv18-20020a17090af19200b0020d2ea64b18mr719028pjb.27.1666056426219; Mon, 17 Oct 2022 18:27:06 -0700 (PDT) Received: from [10.1.1.45] (124-149-170-192.dyn.iinet.net.au. [124.149.170.192]) by smtp.gmail.com with ESMTPSA id 203-20020a6217d4000000b0053653c6b9f9sm7749206pfx.204.2022.10.17.18.27.02 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 17 Oct 2022 18:27:04 -0700 (PDT) Message-ID: Date: Tue, 18 Oct 2022 12:26:59 +1100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Content-Language: en-US To: internals@lists.php.net References: <22177032-fe72-c39b-63fe-fa4368a70852@bastelstu.be> In-Reply-To: <22177032-fe72-c39b-63fe-fa4368a70852@bastelstu.be> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [PHP-DEV] [VOTE] Improve unserialize() error handling From: tstarling@wikimedia.org (Tim Starling) I'm going to vote against exception wrapping. If I'm reading the implementation correctly, the original exception is thrown away, there's no way to get the original message and backtrace. That will make debugging more difficult. I reviewed about 150 callers of unserialize() in the MediaWiki core and extensions. There were no instances of the kind of error guarding shown in the RFC. Occasionally, we have: $result = @unserialize( $data ); if ( !$result ) {      throw new Exception( ... ); } The assumption is that if there is a failure, false will be returned, as is documented in the manual. I don't understand the assertion in the RFC that set_error_handler() is required to detect unserialization failure. Normally, exceptions in MediaWiki are allowed to propagate back to the global exception handler. There are a couple of categories of exceptions which should never be caught as a matter of policy: * Database errors. These are permanent errors. Attempting to persist with executing the request after a database error will mostly likely lead to another error. * Request timeout exceptions. We're using our Excimer extension to implement request timeouts by throwing exceptions from a timer callback. This has been an interesting can of worms which has brought some nice features with a number of unexpected consequences. It means that it's always incorrect to catch and discard an arbitrary Throwable. Probably nothing in our ecosystem is doing database access in an __unserialize() magic method. But if someone tried it, I would want the resulting errors to be properly logged with correct backtraces, not silently discarded. I would vote in favour of such a feature being added as a non-default option, by analogy with JSON_THROW_ON_ERROR. -- Tim Starling Wikimedia Foundation