Use 'ENT_QUOTES|ENT_SUBSTITUTE' for HTML encoding and decoding functions#6583
Closed
craigfrancis wants to merge 4 commits intophp:masterfrom
craigfrancis:master
Closed
Use 'ENT_QUOTES|ENT_SUBSTITUTE' for HTML encoding and decoding functions#6583craigfrancis wants to merge 4 commits intophp:masterfrom craigfrancis:master
craigfrancis wants to merge 4 commits intophp:masterfrom
craigfrancis:master
Conversation
Contributor
|
Sounds like @nikic would be the best person to review this, as he's already familiar with the feature and discussion. |
This reverts commit cf4fd53.
Contributor
Author
|
I've reverted the change that introduced the HTML5 default when decoding. While I think that should be the case, it should be a separate discussion and PR (and I suspect it should not be done via the default flags, because these would be replaced if the programmer specified their own flags without a document type). |
nikic
approved these changes
Jan 18, 2021
Contributor
Author
|
Thanks @nikic |
thekid
added a commit
to xp-forge/mustache
that referenced
this pull request
May 2, 2021
joshhanley
added a commit
to joshhanley/livewire
that referenced
this pull request
Nov 24, 2021
calebporzio
pushed a commit
to livewire/livewire
that referenced
this pull request
Nov 24, 2021
* Add php 8.1 to GitHub actions * Fix htmlspecialchars to have same encoding as PHP 8.1 php/php-src#6583 * Skip Laravel 7 tests for PHP 8.1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Discussion on Internals: https://news-web.php.net/php.internals/112782
Changing the HTML encoding/decoding functions to
ENT_QUOTES|ENT_SUBSTITUTE, because the current default ofENT_COMPATis dangerous:This causes a XSS vulnerability because single quote characters are not encoded by default:
This is why all PHP frameworks I can find use
ENT_QUOTESat a minimum, and will often useENT_SUBSTITUTEto avoid the whole value being lost due to a single bad character.As to
ENT_HTML401, I think that still needs to be the default (rather thanENT_HTML5), as Microsoft Outlook still does not accept', and can be an issue with older versions of Android (both are fine with').Note: This is the first time I've edited any C code for a project (before now I've only edited things for myself), and while it seems to work, I'd appreciate mistakes can be made (feel free to delete). I'm also happy to attempt changes to the doc-en repo as well.