net/textproto: CanonicalMIMEHeaderKey ignores keys with `_` in

[ncw]

textproto.CanonicalMIMEHeaderKey ignores keys with _ in. If an htttp server returns one of these, then it isn't possible to use http.Header.Get to fetch it as it doesn't get canonicalized.

For example, Backblaze use keys with _ in to store metadata - see the X-Bz-Info-src_last_modified_millis in the upload docs.

The Backblaze server returns this header all in lower case. Because it doesn't get canonicalised by textproto.CanonicalMIMEHeaderKey it remains in lower case and consequently `resp.Header.Get("X-Bz-Info-src_last_modified_millis") doesn't find it.

This is easy enough to work around, but caused me a bit of suprise and debugging!

The comment on textproto/reader.go on validHeaderFieldByte explains the problem nicely!

// validHeaderFieldByte reports whether b is a valid byte in a header
// field key. This is actually stricter than RFC 7230, which says:
//   tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." /
//           "^" / "_" / "`" / "|" / "~" / DIGIT / ALPHA
//   token = 1*tchar
// TODO: revisit in Go 1.6+ and possibly expand this. But note that many
// servers have historically dropped '_' to prevent ambiguities when mapping
// to CGI environment variables.
func validHeaderFieldByte(b byte) bool {
    return ('A' <= b && b <= 'Z') ||
        ('a' <= b && b <= 'z') ||
        ('0' <= b && b <= '9') ||
        b == '-'
}

This could cause problems with S3 also which uses a similar scheme for returning user defined metadata too.

[bradfitz]

Oh, sorry, this slipped through everybody's cracks. We've all been so focused on the Go 1.6 dashboard and nobody triaged this for a few days when it came in (oh, Dec 29th), dooming it.

I made a demo here: http://play.golang.org/p/aGv7ijAOx_

In summary, CanonicalMIMEHeaderKey is currently basically this (simplifed):

    func CanonicalMIMEHeaderKey(s string) {
        if strings.Contains(s, "_") { // or other bogus stuff
             return s
        }
        // else actually canonicalize it....
        ...
    }

I think we should probably add underscore as an allowed byte. We'd just not mess with the case of the following the byte like we do for hyphen.

/cc @rsc

Maybe this can still be Go 1.6 material?

[gopherbot]

CL https://golang.org/cl/18725 mentions this issue.

[bradfitz]

Actually, I can just imagine this breaking too many tests. Let's submit at the beginning of Go 1.7 instead.

[ncw]

@bradfitz thanks for looking at this - Go 1.7 sounds fine to me.