Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document XML_OPTION_PARSE_HUGE #3899

Merged
merged 2 commits into from
Oct 23, 2024
Merged

Conversation

nielsdos
Copy link
Member

No description provided.

Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thank you! Maybe add some clarifications, but generally looks good.

Comment on lines 67 to 68
This option should only be enabled when the document size is
bounded.
Copy link
Member

Choose a reason for hiding this comment

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

Probably clear enough, but maybe hint at DoS?

</term>
<listitem>
<simpara>
Available as of PHP 8.4.0.
Copy link
Member

Choose a reason for hiding this comment

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

I have no idea whether libexpat is still properly supported, but if it is, we should clarify here (or maybe below) that the constant is only relevant for libxml2. I'm also not sure whether PHP 7.0 still supported libxml2 < 2.7.0, and if so, what would happen there (would basically XML_OPTION_PARSE_HUGE be enabled by default, and could not be disabled?)

Copy link
Member Author

Choose a reason for hiding this comment

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

you are right; why does it have to be so complex :(

Copy link
Member Author

Choose a reason for hiding this comment

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

Just checked, PHP 7 supports libxml2 2.6.11

Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thank you!

@nielsdos nielsdos merged commit 9acfa18 into php:master Oct 23, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants