Skip to content

Fix GH-20374: PHP with tidy and custom-tags - #20379

Closed
ndossche wants to merge 3 commits into
php:PHP-8.3from
ndossche:fix-20374
Closed

ndossche wants to merge 3 commits into
php:PHP-8.3from
ndossche:fix-20374

Conversation

@ndossche

@ndossche ndossche commented Nov 3, 2025

Copy link
Copy Markdown
Member

Both enums and integers map to TidyInteger, however, in the TidyInteger case we always used zval_get_long(). So for a non-numeric string, this would get turned into 0. 0 is the first enum value in that case, so the wrong enum value could be selected.

To solve this, add special handling for strings and (stringable) objects such that we can explicitly check for numeric strings, and if they're not, handle them as normal strings instead of as 0.

Both enums and integers map to TidyInteger, however, in the TidyInteger
case we always used zval_get_long(). So for a non-numeric string, this
would get turned into 0. 0 is the first enum value in that case, so the
wrong enum value could be selected.

To solve this, add special handling for strings and (stringable) objects
such that we can explicitly check for numeric strings, and if they're
not, handle them as normal strings instead of as 0.
@ndossche ndossche linked an issue Nov 3, 2025 that may be closed by this pull request
Comment thread ext/tidy/tidy.c Resolved
'int' => 1,
'double overflow' => (string) (2.0**80.0),
'numeric string int 1' => '1',
'numeric string double 1.0' => '1.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.

Can I be annoying and ask you to have test cases for INF and NAN? :)

Actually curious what the behaviour would be.

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.

Pushed. Apparently: nothing meaningful as they get turned into (int)0.

@ndossche

ndossche commented Nov 4, 2025

Copy link
Copy Markdown
Member Author

Merged via fcc159b etc

@ndossche ndossche closed this Nov 4, 2025
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.

PHP with tidy and custom-tags

3 participants