From 338e645f20a47cbaca4cfcd9b60670503b75bbc9 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 23 Jul 2015 23:03:13 -0700 Subject: [PATCH] Etags must be quoted according to http spec Signed-off-by: Stephen J Day --- registry/client/repository.go | 11 ++++++----- registry/client/repository_test.go | 2 +- registry/handlers/api_test.go | 11 ++++++----- registry/handlers/images.go | 4 ++-- registry/storage/blobserver.go | 2 +- 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/registry/client/repository.go b/registry/client/repository.go index 29effcce..011bc017 100644 --- a/registry/client/repository.go +++ b/registry/client/repository.go @@ -254,13 +254,14 @@ func (ms *manifests) Get(dgst digest.Digest) (*manifest.SignedManifest, error) { return ms.GetByTag(dgst.String()) } -// AddEtagToTag allows a client to supply an eTag to GetByTag which will -// be used for a conditional HTTP request. If the eTag matches, a nil -// manifest and nil error will be returned. -func AddEtagToTag(tagName, dgst string) distribution.ManifestServiceOption { +// AddEtagToTag allows a client to supply an eTag to GetByTag which will be +// used for a conditional HTTP request. If the eTag matches, a nil manifest +// and nil error will be returned. etag is automatically quoted when added to +// this map. +func AddEtagToTag(tag, etag string) distribution.ManifestServiceOption { return func(ms distribution.ManifestService) error { if ms, ok := ms.(*manifests); ok { - ms.etags[tagName] = dgst + ms.etags[tag] = fmt.Sprintf(`"%s"`, etag) return nil } return fmt.Errorf("etag options is a client-only option") diff --git a/registry/client/repository_test.go b/registry/client/repository_test.go index 232501aa..31e61864 100644 --- a/registry/client/repository_test.go +++ b/registry/client/repository_test.go @@ -430,7 +430,7 @@ func addTestManifestWithEtag(repo, reference string, content []byte, m *testutil Method: "GET", Route: "/v2/" + repo + "/manifests/" + reference, Headers: http.Header(map[string][]string{ - "Etag": {dgst}, + "Etag": {fmt.Sprintf(`"%s"`, dgst)}, }), } diff --git a/registry/handlers/api_test.go b/registry/handlers/api_test.go index 4473eb99..2c6a6003 100644 --- a/registry/handlers/api_test.go +++ b/registry/handlers/api_test.go @@ -429,7 +429,7 @@ func TestBlobAPI(t *testing.T) { checkHeaders(t, resp, http.Header{ "Content-Length": []string{fmt.Sprint(layerLength)}, "Docker-Content-Digest": []string{canonicalDigest.String()}, - "ETag": []string{canonicalDigest.String()}, + "ETag": []string{fmt.Sprintf(`"%s"`, canonicalDigest)}, "Cache-Control": []string{"max-age=31536000"}, }) @@ -440,6 +440,7 @@ func TestBlobAPI(t *testing.T) { t.Fatalf("Error constructing request: %s", err) } req.Header.Set("If-None-Match", etag) + resp, err = http.DefaultClient.Do(req) if err != nil { t.Fatalf("Error constructing request: %s", err) @@ -597,7 +598,7 @@ func TestManifestAPI(t *testing.T) { checkResponse(t, "fetching uploaded manifest", resp, http.StatusOK) checkHeaders(t, resp, http.Header{ "Docker-Content-Digest": []string{dgst.String()}, - "ETag": []string{dgst.String()}, + "ETag": []string{fmt.Sprintf(`"%s"`, dgst)}, }) var fetchedManifest manifest.SignedManifest @@ -619,7 +620,7 @@ func TestManifestAPI(t *testing.T) { checkResponse(t, "fetching uploaded manifest", resp, http.StatusOK) checkHeaders(t, resp, http.Header{ "Docker-Content-Digest": []string{dgst.String()}, - "ETag": []string{dgst.String()}, + "ETag": []string{fmt.Sprintf(`"%s"`, dgst)}, }) var fetchedManifestByDigest manifest.SignedManifest @@ -998,12 +999,12 @@ func checkHeaders(t *testing.T, resp *http.Response, headers http.Header) { for _, v := range vs { if v == "*" { // Just ensure there is some value. - if len(resp.Header[k]) > 0 { + if len(resp.Header[http.CanonicalHeaderKey(k)]) > 0 { continue } } - for _, hv := range resp.Header[k] { + for _, hv := range resp.Header[http.CanonicalHeaderKey(k)] { if hv != v { t.Fatalf("%v header value not matched in response: %q != %q", k, hv, v) } diff --git a/registry/handlers/images.go b/registry/handlers/images.go index e5b0bc77..c1cae4fc 100644 --- a/registry/handlers/images.go +++ b/registry/handlers/images.go @@ -90,13 +90,13 @@ func (imh *imageManifestHandler) GetImageManifest(w http.ResponseWriter, r *http w.Header().Set("Content-Type", "application/json; charset=utf-8") w.Header().Set("Content-Length", fmt.Sprint(len(sm.Raw))) w.Header().Set("Docker-Content-Digest", imh.Digest.String()) - w.Header().Set("Etag", imh.Digest.String()) + w.Header().Set("Etag", fmt.Sprintf(`"%s"`, imh.Digest)) w.Write(sm.Raw) } func etagMatch(r *http.Request, etag string) bool { for _, headerVal := range r.Header["If-None-Match"] { - if headerVal == etag { + if headerVal == etag || headerVal == fmt.Sprintf(`"%s"`, etag) { // allow quoted or unquoted return true } } diff --git a/registry/storage/blobserver.go b/registry/storage/blobserver.go index d0b3204c..36547bcc 100644 --- a/registry/storage/blobserver.go +++ b/registry/storage/blobserver.go @@ -47,7 +47,7 @@ func (bs *blobServer) ServeBlob(ctx context.Context, w http.ResponseWriter, r *h } defer br.Close() - w.Header().Set("ETag", desc.Digest.String()) // If-None-Match handled by ServeContent + w.Header().Set("ETag", fmt.Sprintf(`"%s"`, desc.Digest)) // If-None-Match handled by ServeContent w.Header().Set("Cache-Control", fmt.Sprintf("max-age=%.f", blobCacheControlMaxAge.Seconds())) if w.Header().Get("Docker-Content-Digest") == "" {