imap: lower some fields + content disposition keys #384
Conversation
|
Oh, I also made a few more changes than strictly what was required with my particular issue.
|
|
Option 1 for changing test seems fine to me.
No objection, not sure what would be the practical consideration though.
RFC 3501 is indeed not explicit about this detail, but I assume it is following the same logic of Case-folding these will prevent nasty surprises for naive users that write |
I did this, let me know if there's anything else I missed. |
message.go
Outdated
| @@ -119,6 +119,24 @@ func encodeHeader(s string) string { | |||
| return mime.QEncoding.Encode("utf-8", s) | |||
| } | |||
|
|
|||
| func toLowerTrimmed(s string) string { | |||
| return strings.TrimSpace(strings.ToLower(s)) | |||
There was a problem hiding this comment.
Do we really need to trim spaces here? I haven't encountered fields with spaces in the wild yet.
There was a problem hiding this comment.
Yah I don't know anything about email; wanted to get your guys' thoughts. I removed it.
message.go
Outdated
| } | ||
| if params, ok := disp[1].([]interface{}); ok { | ||
| bs.DispositionParams, _ = parseHeaderParamList(params) | ||
| keysLowered(bs.DispositionParams) |
There was a problem hiding this comment.
I think we can inline the lower-casing in parseHeaderParamList.
There was a problem hiding this comment.
Can we do that in ParseParamList? I think that's the most clean as the map is first populated there, but I didn't want to mess with a public/exported function.
There was a problem hiding this comment.
Yeah, we can't do that because that would be a breaking change.
At least with my mail, a bunch of things in messages, including content-disposition headers, have uppercase keys. This lowercases those strings.
Regarding #383
TestBodyStructure_Formatdoes not pass right now because the BodyStructure contains lowercased values of the input string. So when it's formatted, its field list does not match the input.I could:
bodyStructureTestsinto its own test function that does the equivalent ofTestBodyStructure_Parseon that one case.bodyStructureTestselement struct type to allow overriding the expected value instead offormatFields(test.fields).The first option seems less freaky. I thought I'd ask in case you have better ideas or preferences.