Skip to content

date: Validate nanoseconds range and normalize negative zero duration - #24356

Merged
TimWolla merged 1 commit into
php:PHP-8.6from
adapik:fix-duration-unserialize-nanoseconds
Oct 16, 2026
Merged

TimWolla merged 1 commit into
php:PHP-8.6from
adapik:fix-duration-unserialize-nanoseconds

Conversation

@adapik

@adapik adapik commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #23629 and #23759.

// 1. Previously accepted: nanoseconds is 4294967301 (2^32 + 5), but the
//    internal value is 5, so it compares equal to a 5ns duration.
unserialize('O:13:"Time\Duration":3:{s:7:"seconds";i:0;s:11:"nanoseconds";i:4294967301;s:8:"negative";b:0;}');
// Now: Exception: Invalid serialization data for Time\Duration object

// 2. Previously ->negative stayed true and serialize() round-tripped b:1.
$d = unserialize('O:13:"Time\Duration":3:{s:7:"seconds";i:0;s:11:"nanoseconds";i:0;s:8:"negative";b:1;}');
var_dump($d->negative); // Now: bool(false), like Duration::fromSeconds(0)->negate()

@TimWolla TimWolla left a comment

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.

Good catch, thank you! I have some remarks regarding the actual implementation.

Comment thread ext/date/time_duration.c Outdated
Comment thread ext/date/time_duration.c Outdated
@adapik
adapik force-pushed the fix-duration-unserialize-nanoseconds branch from 8d9a058 to 1abd07b Compare October 16, 2026 04:58
@adapik
adapik requested a review from TimWolla October 16, 2026 04:58
@adapik

adapik commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

Both comments addressed

@TimWolla
TimWolla merged commit ab5e4d7 into php:PHP-8.6 Oct 16, 2026
17 of 18 checks passed
TimWolla added a commit that referenced this pull request Oct 16, 2026
* PHP-8.6:
  date: Validate ISO 8601 duration strings for null bytes early (#24357)
  date: Validate nanoseconds range and reject negative zero duration (#24356)
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.

2 participants