fix(mediatype): quoted semicolon #256
Conversation
internal/stringutil/find.go
Outdated
|
|
||
| // FindQuoted returns the indexes of the instance of v in s, or empty slice if v is not present in s. | ||
| // It ignores v present inside quoted runs. | ||
| func FindQuoted(s string, v rune, quote rune) []int { |
There was a problem hiding this comment.
If I am understanding it's operation correctly, should this be called FindUnquoted? Similar comment on the quotedIndices var name.
There was a problem hiding this comment.
That's true, it sounds like it would find all quoted runes.
The idea was to have the same naming convention. Before this PR, there was a function SplitQuoted, it works like strings.Split, but respects quotes. That's why I named this one FindQuoted, it finds rune, but respects quotes.
I can rename all functions to FindUnquoted, SplitUnquoted and SplitAfterUnqioted, WDYT?
There was a problem hiding this comment.
I think renaming everything to Unquoted would make sense
There was a problem hiding this comment.
fixed 🙂
internal/stringutil/split_test.go
Outdated
| input: `"a;b";"c;d`, | ||
| want: []string{`"a`, `b"`, `"c`, `d`}, | ||
| want: []string{`"a;b"`, `"c`, `d`}, | ||
| }, | ||
| { | ||
| // Quotes must be escaped via RFC2047 encoding, not just a backslash. | ||
| // b through c below must not be treated as a single quoted-run. |
There was a problem hiding this comment.
I think this comment is now out of date, as we are treating b-c as single quoted run. I am a little concerned that we are changing something that is noted as "must" in the comment, but given mediatype tests are still passing, I guess we will roll with it.
There was a problem hiding this comment.
yeah, I was not sure about its original behavior, because it is possible to have Content-Type: application/rtf; charset=utf-8; name="\"žába\".jpg" (in this test), which means quotes can be escaped
There was a problem hiding this comment.
thanks, comment removed
internal/stringutil/split_test.go
Outdated
| }, | ||
| { | ||
| // Quotes must be escaped via RFC2047 encoding, not just a backslash. | ||
| // b through c below must not be treated as a single quoted-run. |
There was a problem hiding this comment.
thanks, comment removed
|
Thank you! |
This PR fixes splitting of Content-Type header, when parameter has semicolon inside quoted text
;, e.g.multipart/mixed; boundary="aaa;bbb;ccc". Previously, it was splitting wrongly, causingfixUnescapedQuotesfunction to break Content-Type header, making itmultipart/mixed; boundary=""Introduce new
FindQuotedfunction. Reasoning: because for the splitting we still need to go through whole string, we can as well find all points where we need split. And then, use it forIndexQuotedorCountQuoted(analog ofstrings.Indexandstrings.Count), if we'll need it the future.