Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:118854 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 36378 invoked from network); 19 Oct 2022 17:22:55 -0000 Received: from unknown (HELO php-smtp4.php.net) (45.112.84.5) by pb1.pair.com with SMTP; 19 Oct 2022 17:22:55 -0000 Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id D9FF61804A7 for ; Wed, 19 Oct 2022 10:22:54 -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,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.2 X-Spam-ASN: AS24940 176.9.0.0/16 X-Spam-Virus: No X-Envelope-From: Received: from chrono.xqk7.com (chrono.xqk7.com [176.9.45.72]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Wed, 19 Oct 2022 10:22:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bastelstu.be; s=mail20171119; t=1666200173; bh=z4Edu5UeO9aRwA+AqcAqiJPtOLTO9tP7T25Uop4Suwo=; h=Date:Subject:To:References:From:In-Reply-To:From; b=idUii9+j/ZIlN9rXLT/+vo6cHByP55aZeELp6WbO5De/ETfBNe/5vR6vYwQQV0XDI 6W13JqJ0R7uiI2ncDGNqNTey7PZJ8V5ADMG502iqR7DEROWD8Wt31iq28LuB79okT6 r2kpxKv6TLcJoi1wcf74F4zkaAeIU3vWZntIB5Ox0v+BXBAljdH2R591DZsnIsPF3K pMGZPSdUh/Y3MviHjJrJhUTGm7OuPdZE2cIb4fQC3ddTbmnNKn77Tpfs2Zn67j954e D+scI1tJSVz2JrWeIgF22CaLE04cG5CeXUubEGbHjr5m662ZiZJ5FsA3i0sWMKVuix 7UYQZMbQmBXUg== Message-ID: <1033db88-adca-82a1-8eb0-6e18e894e96b@bastelstu.be> Date: Wed, 19 Oct 2022 19:22:52 +0200 MIME-Version: 1.0 Content-Language: en-US To: Tim Starling , internals@lists.php.net References: <22177032-fe72-c39b-63fe-fa4368a70852@bastelstu.be> <5d5849c5-ca79-71e9-ccc8-5c69952fd408@bastelstu.be> <312c7bee-2258-d4f4-a53d-892f5c66a07f@wikimedia.org> In-Reply-To: <312c7bee-2258-d4f4-a53d-892f5c66a07f@wikimedia.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [PHP-DEV] [VOTE] Improve unserialize() error handling From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=c3=bcsterhus?=) Hi On 10/19/22 06:14, Tim Starling wrote: >> The example in the RFC was written to be correct for the general >> case, without imposing any constraints in the input data. > > As I said, if throwing was optional, like in json_decode(), I would be > all for it. I'm just doing a cost/benefit analysis. The common case > should be easy, while the general case should be possible. > > As a matter of style, I think PHP's false returns are fine. I don't > think we need to follow Python into making every error an exception. > Let me clarify why unserialize is not really comparable to json_decode: The unserialize handlers (__unserialize, __wakeup, Serializable::unserialize()) effectively allow arbitrary (both native and userland) code to run during unserialize(). This arbitrary code might or might not throw an Exception. Thus you already need to be prepared to deal with Exceptions being thrown in the general case: Whenever you unserialize objects you didn't write yourself. You don't know if a library introduces a change that causes any existing long-lived serialized object to become incompatible. --------- Here's an example I've already sent to the list during the discussion phase of this RFC: https://3v4l.org/Xsmfu The readonly class 'Foo' had a private (!) '$a' property in the initial version. This property was removed in a future version, thus breaking all serialized versions of it, unless the author makes sure to add an unserialize handler that handles this gracefully. As a developer you generally wouldn't consider a change to a private property to be a breaking API change, making this an unexpected gotcha. --------- Thus *not setting* "UNSERIALIZE_THROW_ON_ERROR" (or whatever you may want to call it) *does not* guarantee that unserialize() won't throw. You need to deal with both Notice/Warning and Exceptions. On the other hand if the Notice/Warning becomes an Exception in a future version the error handling is simplified: You only need to deal with Exceptions (specifically a single type of Exception with this RFC). Best regards Tim Düsterhus