fix(gmail): pre-wrap quoted lines to prevent QP soft-wrap corruption in +reply#833
Conversation
🦋 Changeset detectedLatest commit: 4c997a7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where long lines in quoted email replies could cause corruption due to Quoted-Printable soft-wrapping. By implementing a pre-wrapping mechanism for quoted text, the changes ensure that lines remain within safe length limits, improving the reliability of email formatting in the CLI. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a line-wrapping mechanism to prevent Quoted-Printable soft-wrap corruption in Gmail replies by wrapping quoted lines to a maximum of 73 characters before adding the "> " prefix. It also includes corresponding unit tests. Feedback points out that the current wrap_line implementation fails to wrap individual words (such as long URLs or base64 strings) that exceed the maximum length, which would still trigger the soft-wrap issue. A robust solution is suggested to hard-wrap these long words at safe UTF-8 boundaries.
| fn wrap_line(line: &str, max_len: usize) -> Vec<String> { | ||
| if line.len() <= max_len { | ||
| return vec![line.to_string()]; | ||
| } | ||
| // word-wrap at max_len, breaking on spaces | ||
| let mut result = Vec::new(); | ||
| let mut current = String::new(); | ||
| for word in line.split(' ') { | ||
| if current.is_empty() { | ||
| current.push_str(word); | ||
| } else if current.len() + 1 + word.len() <= max_len { | ||
| current.push(' '); | ||
| current.push_str(word); | ||
| } else { | ||
| result.push(current.clone()); | ||
| current = word.to_string(); | ||
| } | ||
| } | ||
| if !current.is_empty() { | ||
| result.push(current); | ||
| } | ||
| result | ||
| } |
There was a problem hiding this comment.
The current implementation of wrap_line does not wrap individual words that are longer than max_len. If an email contains a long unbroken string (such as a URL, a base64 block, or a long path), line.split(' ') will produce a single word exceeding max_len. Since the function does not split words longer than max_len, the line will remain unwrapped and will still exceed the SMTP/MIME 76-character limit once prefixed with > . This will trigger the exact Quoted-Printable soft-wrap corruption this PR aims to prevent.
To fix this, we should hard-wrap words longer than max_len at safe UTF-8 character boundaries.
fn wrap_line(line: &str, max_len: usize) -> Vec<String> {
if line.len() <= max_len {
return vec![line.to_string()];
}
let mut result = Vec::new();
let mut current = String::new();
for word in line.split(' ') {
if word.len() > max_len {
if !current.is_empty() {
result.push(current.clone());
current.clear();
}
let mut start = 0;
while start < word.len() {
let mut end = start + max_len;
if end >= word.len() {
current = word[start..].to_string();
break;
}
while !word.is_char_boundary(end) {
end -= 1;
}
if end == start {
end = start + 1;
while end < word.len() && !word.is_char_boundary(end) {
end += 1;
}
}
result.push(word[start..end].to_string());
start = end;
}
} else {
if current.is_empty() {
current.push_str(word);
} else if current.len() + 1 + word.len() <= max_len {
current.push(' ');
current.push_str(word);
} else {
result.push(current.clone());
current = word.to_string();
}
}
}
if !current.is_empty() {
result.push(current);
}
result
}References
- Avoid introducing changes that are outside the primary goal of a pull request to prevent scope creep.
Description