Fix #53891: iconv_mime_encode() fails to Q-encode UTF-8 string #3436
Conversation
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
The minimum length of an encoded-word is actually the pure encoding overhead plus the length of the `output-charset` plus the minimum unit of encoded text, which is 4 for B-encoding and (for simplicity) 3 for Q-encoding. We also cater to the possibility that we need further encoded words, which would be split by the `line-break-chars` followed by a space character. Obviously, the former `out_charset_len + 12` is too simplistic and wrong in the given case (where the magic number would be 13). These simplifications are somewhat wasteful, but iconv_mime_encode() with Q-encoding is wasteful anyway (see bug 66828[1]), and the proper solution to convert the whole input to the desired output charset upfront, and applying the encoding afterwards appears too much a change for the stable releases. [1] <https://bugs.php.net/66828>
Member
Author
|
I'm not sure whether this fix would have unintended side effects, so unless there are objections, I'll skip the stable branches, and apply this patch to PHP-7.3+ only on the weekend. |
|
Comment on behalf of cmb at php.net: Applied via eb03290. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The minimum length of an encoded-word is actually the pure encoding
overhead plus the length of the
output-charsetplus the minimum unitof encoded text, which is 4 for B-encoding and (for simplicity) 3 for
Q-encoding. We also cater to the possibility that we need further
encoded words, which would be split by the
line-break-charsfollowedby a space character. Obviously, the former
out_charset_len + 12istoo simplistic and wrong in the given case (where the magic number
would be 13).
These simplifications are somewhat wasteful, but iconv_mime_encode()
with Q-encoding is wasteful anyway (see bug 66828[1]), and the proper
solution to convert the whole input to the desired output charset
upfront, and applying the encoding afterwards appears too much a change
for the stable releases.
[1] https://bugs.php.net/66828