Skip to content

PHP 8.3: Improve unserialize() error handling #1988

@afilina

Description

@afilina

Related to #1589 and #1987

Analysis

The RFC had 3 proposals:

  • Add wrapper Exception 'UnserializationFailedException' in PHP 8.3: ❌ did not pass
  • Increase the error reporting severity in the unserialize() parser in PHP 8.3: ✅ passed
  • Throw Exception instead of emitting warning / notice in PHP 9.0: ✅ passed (but targets next major version, out of scope for this analysis)

Summary of existing PHP 8.2 behavior based on the RFC:

unserialize('foo'); // Notice
unserialize('i:12345678901234567890;'); // Warning
unserialize('E:3:"foo";'); // Warning AND Notice (2 separate issues with the input)
unserialize('E:3:"fo:";'); // Warning AND Notice (2 separate issues with the input)

Updated behavior in PHP 8.3:

unserialize('foo'); // Warning
unserialize('i:12345678901234567890;'); // Warning
unserialize('E:3:"foo";'); // Warning x 2
unserialize('E:3:"fo:";'); // Warning x 2

Top 2000 Packages

Found thousands of occurrences, but only 2 where input can be reliably known:

  • drupal/core - plain serialized string in a test
  • magento/magento-coding-standard - empty string in a test

Source: https://gist.github.com/afilina/d385de0733d0293fd4067824bae60b34

Detection in PHP 8.2

  • unserialize('foo'); - Error at offset: invalid input, emits notice
  • unserialize('i:12345678901234567890;'); - Numerical result out of range: invalid input, emits warning
  • unserialize('E:3:"foo";'); - Invalid enum name & error at offset: invalid input, emits warning & notice
  • unserialize('E:3:"fo:";'); - Class not found & error at offset: invalid input, emits warning & notice

Detection in PHP 8.3

  • unserialize('foo'); - Error at offset: invalid input, emits warning
  • unserialize('i:12345678901234567890;'); - Numerical result out of range: invalid input, emits warning
  • unserialize('E:3:"foo";'); - Invalid enum name & error at offset: invalid input, emits warning for each
  • unserialize('E:3:"fo:";'); - Class not found & error at offset: invalid input, emits warning for each

Syntax Variations & Detectability

See "Syntax Variations & Detectability" in a related ticket: #1987

Invalid input - notices promoted to warnings

I don't think there's anything to sniff, since it's just notices vs warnings with input being invalid in both versions. The PHPCompatibility output would be the same: warning. Perhaps it was never sniffed in the first place because of the feasibility of such a sniff due to the need to find and interpret the input semantically? Other than using unserialize() itself in the sniff, of course.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions