Skip to content

Do not split UTF-8 characters between "Q"-encoded-words#41

Merged
mdecimus merged 3 commits into
stalwartlabs:mainfrom
link2xt:link2xt/quoted-printable-utf8-split
Jul 26, 2025
Merged

Do not split UTF-8 characters between "Q"-encoded-words#41
mdecimus merged 3 commits into
stalwartlabs:mainfrom
link2xt:link2xt/quoted-printable-utf8-split

Conversation

@link2xt

@link2xt link2xt commented Jul 25, 2025

Copy link
Copy Markdown
Contributor

Partially addresses #40.

Multiple commits, don't squash.

@link2xt

link2xt commented Jul 25, 2025

Copy link
Copy Markdown
Contributor Author

This does not enforce 76 character limit strictly and only splits newlines after overflow, but is an improvement over current state anyway. The test does not rely on exact line lengths, so this can be improved later and the test will ensure that there is no regression wrt. fix for character boundaries.

@link2xt link2xt force-pushed the link2xt/quoted-printable-utf8-split branch from 2dac05c to 4eb118e Compare July 25, 2025 21:12
Comment thread src/headers/text.rs Outdated
bytes_written += prefix.len();

for (pos, &ch) in self.text.as_bytes().iter().enumerate() {
if bytes_written >= 76 && self.text.is_char_boundary(pos) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would be more idiomatic (and likely more performant) to replace the current self.text.as_bytes().iter().enumerate() + text.is_char_boundary() combination with self.text.char_indices().

char_indices() allows iterating over valid UTF-8 character boundaries and avoids the need to manually check boundaries using is_char_boundary on each iteration.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I thought about it, but I want to iterate over bytes rather than characters. Internally char_indices() just uses chars() iterator.

Regarding performance, checking for UTF-8 boundary is very cheap, it just looks at the byte value internally using this private function:
https://github.com/rust-lang/rust/blob/8708f3cd1f96d226f6420a58ebdd61aa0bc08b0a/library/core/src/num/mod.rs#L1073-L1077
Iterating over bytes might actually be more performant than iterating to the end of char and then rewinding back and looping over all char bytes again, but in any case I think it's better to not do such low-level optimizations without measurements.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In that case you can replace:

self.text.is_char_boundary(pos)

with

pos == 0 || ch.is_utf8_char_boundary()

this avoids the extra array access done internally by self.text.is_char_boundary(pos).

@link2xt link2xt Jul 26, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

is_utf8_char_boundary is private. And is_char_boundary is inlined, so compiler should optimize array access.
See also comments in the standard library:
https://github.com/rust-lang/rust/blob/8708f3cd1f96d226f6420a58ebdd61aa0bc08b0a/library/core/src/str/mod.rs#L360-L383
Anyway, I don't see much point in optimizing the code without measurements. I will be likely changed in the future anyway to have strict 76 character boundary, which I don't try to fix. My only goal is to fix the bug with UTF-8 characters being broken between encoded-words.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, is_utf8_char_boundary is inlined but self.text.is_char_boundary(pos) is accessing the array by pos on each iteration and you already have ch with the value needed.

If is_utf8_char_boundary is private then an alternative would be pos == 0 || (ch as i8) >= -0x40.

@link2xt link2xt force-pushed the link2xt/quoted-printable-utf8-split branch from 4eb118e to dbb451d Compare July 26, 2025 16:33
@link2xt link2xt force-pushed the link2xt/quoted-printable-utf8-split branch from dbb451d to 8825292 Compare July 26, 2025 16:36
@mdecimus mdecimus merged commit b02e444 into stalwartlabs:main Jul 26, 2025
1 check passed
@mdecimus

Copy link
Copy Markdown
Member

Thanks!

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