From 8907f7d1899a578e3e5d10ec34ec49f1841fb154 Mon Sep 17 00:00:00 2001 From: Tianon Gravi Date: Fri, 10 Jun 2016 16:34:08 -0700 Subject: [PATCH] Update "Accept" header parsing for list values In Go's header parsing, the same header multiple times results in multiple entries in the `r.Header[...]` slice, but Go does no further parsing beyond that (and in https://golang.org/cl/4528086 it was determined that until/unless the stdlib itself needs it, Go will not do so). The consequence here for parsing of `Accept:` headers is that we support the way Go outputs headers, but not all language HTTP libraries have a facility to output multiple headers instead of a single list header. This change ensures that the following (valid) header blocks all parse to the same result for the purposes of what is being tested here: ``` Accept: a/b Accept: b/c Accept: d/e ``` ``` Accept: a/b; q=0.5, b/c Accept: d/e ``` ``` Accept: a/b; q=0.1, b/c; q=0.2, d/e; q=0.8 ``` Signed-off-by: Andrew "Tianon" Page --- registry/handlers/api_test.go | 4 ++-- registry/handlers/images.go | 20 ++++++++++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/registry/handlers/api_test.go b/registry/handlers/api_test.go index 076207ed2..93585d45f 100644 --- a/registry/handlers/api_test.go +++ b/registry/handlers/api_test.go @@ -1586,8 +1586,8 @@ func testManifestAPIManifestList(t *testing.T, env *testEnv, args manifestArgs) if err != nil { t.Fatalf("Error constructing request: %s", err) } - req.Header.Set("Accept", manifestlist.MediaTypeManifestList) - req.Header.Add("Accept", schema1.MediaTypeSignedManifest) + // multiple headers in mixed list format to ensure we parse correctly server-side + req.Header.Set("Accept", fmt.Sprintf(` %s ; q=0.8 , %s ; q=0.5 `, manifestlist.MediaTypeManifestList, schema1.MediaTypeSignedManifest)) req.Header.Add("Accept", schema2.MediaTypeManifest) resp, err = http.DefaultClient.Do(req) if err != nil { diff --git a/registry/handlers/images.go b/registry/handlers/images.go index dd2ed2c84..df7f869be 100644 --- a/registry/handlers/images.go +++ b/registry/handlers/images.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "net/http" + "strings" "github.com/docker/distribution" ctxu "github.com/docker/distribution/context" @@ -98,8 +99,23 @@ func (imh *imageManifestHandler) GetImageManifest(w http.ResponseWriter, r *http supportsSchema2 := false supportsManifestList := false - if acceptHeaders, ok := r.Header["Accept"]; ok { - for _, mediaType := range acceptHeaders { + // this parsing of Accept headers is not quite as full-featured as godoc.org's parser, but we don't care about "q=" values + // https://github.com/golang/gddo/blob/e91d4165076d7474d20abda83f92d15c7ebc3e81/httputil/header/header.go#L165-L202 + for _, acceptHeader := range r.Header["Accept"] { + // r.Header[...] is a slice in case the request contains the same header more than once + // if the header isn't set, we'll get the zero value, which "range" will handle gracefully + + // we need to split each header value on "," to get the full list of "Accept" values (per RFC 2616) + // https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.1 + for _, mediaType := range strings.Split(acceptHeader, ",") { + // remove "; q=..." if present + if i := strings.Index(mediaType, ";"); i >= 0 { + mediaType = mediaType[:i] + } + + // it's common (but not required) for Accept values to be space separated ("a/b, c/d, e/f") + mediaType = strings.TrimSpace(mediaType) + if mediaType == schema2.MediaTypeManifest { supportsSchema2 = true }