Re: [PHP-DEV] [VOTE] Improve unserialize() error handling
Hi
On 10/14/22 22:42, Nicolas Grekas wrote:
Not sure why I didn't think about it before but I just ran the test suite
of Symfony after applying the patch proposed in the RFC to change the way
exceptions are handled by unserialize.
This change breaks the test suite of 5 separate components. I created this
gist to list all the failures:
https://gist.github.com/nicolas-grekas/3da652a51669baa40c99bd20e4a1b4dd
Maybe I wasn't convincing enough during the discussion period, but that
doesn't change the fact that the proposed behavior in the RFC is a very
clear BC break that will affect userland significantly.
I'm therefore voting NO on the proposal.
I'm not surprised by the “no” on the first vote based on the previous
discussion. I am surprised however that you also disagree with raising
the E_NOTICE to E_WARNING for consistency.
I don't plan on repeating all the previous discussion points with you,
but as you mention the Symfony tests specifically: Please find the patch
attached. Do you believe the expectations in this new test would a
reasonable expectation by a Symfony user?
Best regards
Tim Düsterhusdiff --git i/src/Symfony/Component/Messenger/Tests/Transport/Serialization/PhpSerializerTest.php w/src/Symfony/Component/Messenger/Tests/Transport/Serialization/PhpSerializerTest.php
index c83606a59f..609187 100644
--- i/src/Symfony/Component/Messenger/Tests/Transport/Serialization/PhpSerializerTest.php
+++ w/src/Symfony/Component/Messenger/Tests/Transport/Serialization/PhpSerializerTest.php
@@ -77,6 +77,17 @@ class PhpSerializerTest extends TestCase
]);
}
+public function testDecodingFailsWithBadData()
+{
+$this->expectException(MessageDecodingFailedException::class);
+
+$serializer = $this->createPhpSerializer();
+
+$serializer->decode([
+'body' => 'O:8:"DateTime":0:{}',
+]);
+}
+
public function testEncodedSkipsNonEncodeableStamps()
{
$serializer = $this->createPhpSerializer();
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [VOTE] Improve unserialize() error handling
Hi Tim, as announced on Wednesday [1] I've now opened the vote for: > > "Improve unserialize() error handling" [2] > > The RFC contains three votes, each of which requires a 2/3 majority. Two > of the votes are for 8.x (8.3), one for 9.0. > > Voting will run 2 weeks until: > > 2022-10-28 at 14:00 UTC > > > > Please find the below resources for your reference: > > RFC: https://wiki.php.net/rfc/improve_unserialize_error_handling > PoC implementation: > - https://github.com/php/php-src/pull/9425 > - https://github.com/php/php-src/pull/9629 > > Discussion Thread: https://externals.io/message/118566 > Related Thread: https://externals.io/message/118311 > > > > [1] https://externals.io/message/118566#118807 > [2] https://wiki.php.net/rfc/improve_unserialize_error_handling > Not sure why I didn't think about it before but I just ran the test suite of Symfony after applying the patch proposed in the RFC to change the way exceptions are handled by unserialize. This change breaks the test suite of 5 separate components. I created this gist to list all the failures: https://gist.github.com/nicolas-grekas/3da652a51669baa40c99bd20e4a1b4dd Maybe I wasn't convincing enough during the discussion period, but that doesn't change the fact that the proposed behavior in the RFC is a very clear BC break that will affect userland significantly. I'm therefore voting NO on the proposal. Cheers, Nicolas
[PHP-DEV] [VOTE] Improve unserialize() error handling
Hi as announced on Wednesday [1] I've now opened the vote for: "Improve unserialize() error handling" [2] The RFC contains three votes, each of which requires a 2/3 majority. Two of the votes are for 8.x (8.3), one for 9.0. Voting will run 2 weeks until: 2022-10-28 at 14:00 UTC Please find the below resources for your reference: RFC: https://wiki.php.net/rfc/improve_unserialize_error_handling PoC implementation: - https://github.com/php/php-src/pull/9425 - https://github.com/php/php-src/pull/9629 Discussion Thread: https://externals.io/message/118566 Related Thread: https://externals.io/message/118311 [1] https://externals.io/message/118566#118807 [2] https://wiki.php.net/rfc/improve_unserialize_error_handling Best regards Tim Düsterhus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
