Use 'ENT_QUOTES|ENT_SUBSTITUTE' for HTML encoding and decoding functions

[craigfrancis]

I'm not familiar with the PHP development process, but it looks like PR 6583 has been accepted in 50eca61, and will be in PHP 8.1.

These should update the documentation to reflect this change.

[Girgias]

Thanks for the PR, this will not be merged until we're closer to the PHP 8.1 release as per the current doc process.

Might need to be rebased in 6-8 months time.

[craigfrancis]

Yep, np, and feel free to edit/delete if it's not right.

[afilina]

@Girgias Is there a "pending milestone" tag or something along those lines to make it more apparent in the PR list? Sorry to be a bother, it's just that it makes it harder to sort all the open PRs when I go through the list periodically to try and help PRs along.

[salathe]

Did these changes deliberately exclude ENT_HTML401 from the default values, or just an oversight (the files "before" also didn't reference it in the prototypes but did in the descriptions, for the most part)?

[cmb69]

@salathe, nor do the stubs specify ENT_HTML401, likely because its value is 0 so hasn't any effect on the bitmask.

[craigfrancis]

I'm fairly sure ENT_HTML401 is the default (value 0).

We did consider using ENT_HTML5 due to feedback from Claude Pache, and I did try adding it to the patch, but removed it to keep the change small, and because I felt that should have had a bit more of a discussion (which I don't think happened)... my main focus was in getting ENT_QUOTES included, due to the security issues it can cause:

$url = "/' onerror='alert(1)";
echo "<img src='" . htmlspecialchars($url) . "'>";

[cmb69]

I'm fairly sure ENT_HTML401 is the default (value 0).

Yes, and it might be good to add it (also to the stubs) for clarity. However, ENT_QUOTES|ENT_HTML401 === ENT_QUOTES (etc.)

[salathe]

Thanks @craigfrancis

Apologies for not looking at this sooner and raising the inconsistency while the PR was still open.

Going forward, I think the docs should have the default value be at least consistent within each page, even if ENT_QUOTES | ENT_SUBSTITUTE and ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML401 are really one and the same set of bits. My preference is to align on using ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML401.

Thoughts, violent disagreement, volunteers?

[craigfrancis]

I have a slight preference to including ENT_HTML401 to note that it's the default.

Some developers might see that and then decide to use ENT_HTML5, even though ' still upsets Microsoft Outlook (yep, it's been well over 13 years).

If everyone is happy with that, are you in a better position to make that change? If it's a pain, let me know, and I should be able to create a new pull-request on the weekend.

[cmb69]

But please fix the stubs first (otherwise this is likely to be overwritten later): https://github.com/php/php-src/blob/c37b35fa41be7ec4278e51c6b6715a8805ebb725/ext/standard/basic_functions.stub.php#L686

[craigfrancis]

I've created a pull-request to update the stubs: php/php-src#7514

[craigfrancis]

The stubs have been updated - Thanks to @nikic who finished it off, and merged it it via 2b25ac6.