Skip to content

standard: Pass unserialize('') through the regular path - #23827

Merged
TimWolla merged 2 commits into
php:PHP-8.6from
TimWolla:unserialize-empty-warn
Sep 22, 2026
Merged

TimWolla merged 2 commits into
php:PHP-8.6from
TimWolla:unserialize-empty-warn

Conversation

@TimWolla

Copy link
Copy Markdown
Member

Previously unserialize() with an empty string as input was handled specially, returning false (indicating an error), but without emitting an associated warning. We can just pass it through the regular path, which will also return false (including the warning) and will also avoid a check that is almost never taken in practice.

Fixes #23780.

Previously `unserialize()` with an empty string as input was handled specially,
returning `false` (indicating an error), but without emitting an associated
warning. We can just pass it through the regular path, which will also return
`false` (including the warning) and will also avoid a check that is almost
never taken in practice.

Fixes php#23780.
Comment thread ext/standard/var.c
HashTable *class_hash = NULL, *prev_class_hash;
zend_long prev_max_depth, prev_cur_depth;

if (buf_len == 0) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rings a bell though. I think I recall a case somewhere in the codebase wshere a non-null terminated string was passed to unserialize; that might need fixing (if it's still there and my memory serves me well) before proceeding with this.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

php_unserialize_with_options() is only ever called from:

  • phar_metadata_tracker_unserialize_or_copy()
  • PHP_FUNCTION(unserialize)

in both cases the input is a zend_string which would already be malformed when not terminated. And php_var_unserialize_internal() also immediately bails out when YYCURSOR >= YYLIMIT, which is the case for an empty input.

So this should be safe.

@TimWolla
TimWolla changed the base branch from master to PHP-8.6 September 22, 2026 16:09
@TimWolla
TimWolla merged commit 2628651 into php:PHP-8.6 Sep 22, 2026
18 checks passed
TimWolla added a commit that referenced this pull request Sep 22, 2026
* PHP-8.6:
  standard: Pass `unserialize('')` through the regular path (#23827)
@TimWolla
TimWolla deleted the unserialize-empty-warn branch September 22, 2026 17:02
sebastianbergmann added a commit to sebastianbergmann/phpunit that referenced this pull request Sep 23, 2026
As of php/php-src#23827, unserialize('') emits a warning, which our error handler converts to an exception
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Possibly missing warning for unserialize('') - may be confused with "false"

2 participants