Unescape quoted-pairs inside quoted-strings - #140
Conversation
Parameter values in Content-Type/Content-Disposition had their surrounding quotes stripped but the \" and \\ pairs inside them left as-is, so filename="a\"b.txt" came back as a\"b.txt. Two related spots: Display for SingleInfo/GroupInfo escaped " in a display name but not \, so a name holding a backslash round-tripped through addrparse with the backslash dropped; and the comment state ended on an escaped \), leaking the rest of the comment into the parsed address. Display names use the full RFC 5322 quoted-pair, matching what the quoted-name reader already does. Parameter values only undo \" and \\, which keeps the unescaped Windows paths that show up in real filename= parameters intact, the same line Python's email and Go's mime draw.
| let inner = &value[1..value.len() - 1]; | ||
| let mut out = String::with_capacity(inner.len()); | ||
| let mut chars = inner.chars().peekable(); | ||
| while let Some(c) = chars.next() { | ||
| if c == '\\' && matches!(chars.peek(), Some('"') | Some('\\')) { | ||
| out.push(chars.next().unwrap()); | ||
| } else { | ||
| out.push(c); | ||
| } | ||
| } |
There was a problem hiding this comment.
If value ends with \" then this is going to leave the trailing backslash (because chars.peek() will be None), is that desirable? e.g.
assert_eq!(unquote_param_value(r#""foo\""#), r#"foo\"#.to_string());
should this produce a parsing error instead?
There was a problem hiding this comment.
Good catch — that trailing backslash was wrong either way. Rather than erroring (this fn isn't fallible and I didn't want to change the public signature), I made it detect that case: if the closing " is preceded by an odd run of backslashes, it's the escaped half of a \" pair, not a real delimiter, so the value is an unterminated quoted-string. Now it's left completely untouched instead of having its last backslash silently eaten:
assert_eq!(unquote_param_value(r#""foo\""#), r#""foo\""#.to_string());Added a test for it. Existing well-formed cases (e.g. "a\\" -> a\) are unaffected since the parity check only fires on the pathological unterminated case.
2 participants
parse_content_disposition(r#"attachment; filename="a\"b.txt""#)returns the filename asa\"b.txtinstead ofa"b.txt— the quotes around a parameter value get stripped but the quoted-pairs inside them are never undone. Two neighbouring spots have the same gap:DisplayforSingleInfo/GroupInfoescapes"in a display name but not\, so the nameFoo\Barrenders as"Foo\Bar" <x@y.com>andaddrparsereads that straight back asFooBar— the crate emits something its own parser silently corrupts; and the comment state ends on an escaped\), sox@y.com (a\)b)parses to the addressx@y.com b). The quoted display-name reader is the one place that already gets this right, which is why the round-trip loses data rather than erroring.In display names I kept RFC 5322 §3.2.1
quoted-pairin full (backslash before anything,obs-qpincluded) since that is what the quoted-name reader already does. In MIME parameter values I only undo\"and\\and leave every other\xalone, sofilename="C:\dev\go\foo.txt"still comes out intact — that is where Python'semailand Go'smimeboth draw the line, and I checked each row of the change against both (the only case they disagree on is\;, which I left untouched). A backslash outside quotes stays literal either way; the existingparse_backslashestest fails if that gets widened.cargo test --allgoes from 54 to 56 unit tests plus 25 doctests, all green;cargo fmt -- --checkis clean andcargo clippyshows no new warnings.