Skip to content

[Mime] fix encoding issue with UTF-8 addresses containing doubles spaces #58593

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 23, 2024
Merged

[Mime] fix encoding issue with UTF-8 addresses containing doubles spaces #58593

merged 1 commit into from
Oct 23, 2024

Conversation

0xb4lint
Copy link
Contributor

@0xb4lint 0xb4lint commented Oct 17, 2024 •
edited by nicolas-grekas
Loading

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #58592, Fix #50345
License MIT

@carsonbot carsonbot added this to the 7.2 milestone Oct 17, 2024
@OskarStark OskarStark requested a review from xabbuh October 17, 2024 15:05
@nicolas-grekas nicolas-grekas modified the milestones: 7.2, 5.4 Oct 23, 2024
@nicolas-grekas nicolas-grekas changed the title [Mime] fix encoding issue with UTF-8 addresses containing doubles spaces + tests [Mime] fix encoding issue with UTF-8 addresses containing doubles spaces Oct 23, 2024
@symfony symfony deleted a comment from carsonbot Oct 23, 2024
nicolas-grekas
nicolas-grekas reviewed Oct 23, 2024
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

LGTM but would there be a way to do this in one go in one foreach loop somehow?

@0xb4lint
Copy link
Contributor Author

@nicolas-grekas that was my first intention, but I've realized it would result a really messy code (without doing a significant refactor)

performance-wise, I conducted some benchmarks by calling (new MailboxHeader(...))->getBodyAsString()
1M iteration without the second loop: 26.45s - 26.57s
1M iteration with the second loop: 25.47s - 26.14s

@fabpot fabpot changed the base branch from 7.2 to 5.4 October 23, 2024 20:18
@fabpot fabpot force-pushed the issue/mime-utf8-double-spaces branch from 4f95d06 to c6ed3c8 Compare October 23, 2024 20:18
@fabpot
Copy link
Member

fabpot commented Oct 23, 2024

Thank you @0xb4lint.

@fabpot fabpot merged commit fa18010 into symfony:5.4 Oct 23, 2024
10 of 12 checks passed
This was referenced Oct 27, 2024
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@xabbuh xabbuh Awaiting requested review from xabbuh

@lyrixx lyrixx Awaiting requested review from lyrixx

@yceruto yceruto Awaiting requested review from yceruto

@chalasr chalasr Awaiting requested review from chalasr

@dunglas dunglas Awaiting requested review from dunglas

Assignees
No one assigned
Labels
Bug Mime Status: Needs Review
Projects
None yet
Milestone
5.4
4 participants
@0xb4lint @fabpot @nicolas-grekas @carsonbot