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üsterhus
diff --git i/src/Symfony/Component/Messenger/Tests/Transport/Serialization/PhpSerializerTest.php w/src/Symfony/Component/Messenger/Tests/Transport/Serialization/PhpSerializerTest.php
index c83606a59f..6091879999 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
