fix conformance failure (#15261)

fixes #15252

Give 404 for invalid digest when to get/head manifest

Signed-off-by: Wang Yan <wangyan@vmware.com>
This commit is contained in:
Wang Yan 2021-07-06 15:11:13 +08:00 committed by GitHub
parent ddb6619769
commit b158086642
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 44 additions and 13 deletions

View File

@ -19,7 +19,7 @@ const (
var ( var (
// V2ManifestURLRe is the regular expression for matching request v2 handler to view/delete manifest // V2ManifestURLRe is the regular expression for matching request v2 handler to view/delete manifest
V2ManifestURLRe = regexp.MustCompile(fmt.Sprintf(`^/v2/(?P<%s>%s)/manifests/(?P<%s>%s|%s)$`, RepositorySubexp, reference.NameRegexp.String(), ReferenceSubexp, reference.TagRegexp.String(), digest.DigestRegexp.String())) V2ManifestURLRe = regexp.MustCompile(fmt.Sprintf(`^/v2/(?P<%s>%s)/manifests/(?P<%s>.*)$`, RepositorySubexp, reference.NameRegexp.String(), ReferenceSubexp))
// V2TagListURLRe is the regular expression for matching request to v2 handler to list tags // V2TagListURLRe is the regular expression for matching request to v2 handler to list tags
V2TagListURLRe = regexp.MustCompile(fmt.Sprintf(`^/v2/(?P<%s>%s)/tags/list`, RepositorySubexp, reference.NameRegexp.String())) V2TagListURLRe = regexp.MustCompile(fmt.Sprintf(`^/v2/(?P<%s>%s)/tags/list`, RepositorySubexp, reference.NameRegexp.String()))
// V2BlobURLRe is the regular expression for matching request to v2 handler to retrieve head/delete a blob // V2BlobURLRe is the regular expression for matching request to v2 handler to retrieve head/delete a blob

View File

@ -21,12 +21,18 @@ import (
) )
func TestMatchManifestURLPattern(t *testing.T) { func TestMatchManifestURLPattern(t *testing.T) {
_, _, ok := MatchManifestURLPattern("") _, _, ok := MatchManifestURLPattern("/v2/library/hello-world/manifests/.Invalid")
assert.False(t, ok) assert.True(t, ok)
_, _, ok = MatchManifestURLPattern("/v2/") _, _, ok = MatchManifestURLPattern("/v2/")
assert.False(t, ok) assert.False(t, ok)
_, _, ok = MatchManifestURLPattern("/v2/library/hello-world/manifests//")
assert.True(t, ok)
_, _, ok = MatchManifestURLPattern("/v2/library/hello-world/manifests/###")
assert.True(t, ok)
repository, reference, ok := MatchManifestURLPattern("/v2/library/hello-world/manifests/latest") repository, reference, ok := MatchManifestURLPattern("/v2/library/hello-world/manifests/latest")
assert.True(t, ok) assert.True(t, ok)
assert.Equal(t, "library/hello-world", repository) assert.Equal(t, "library/hello-world", repository)

View File

@ -16,6 +16,7 @@ package artifactinfo
import ( import (
"fmt" "fmt"
"github.com/docker/distribution/reference"
lib_http "github.com/goharbor/harbor/src/lib/http" lib_http "github.com/goharbor/harbor/src/lib/http"
"net/http" "net/http"
"net/url" "net/url"
@ -50,7 +51,11 @@ func Middleware() func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
log.Debugf("In artifact info middleware, url: %s", req.URL.String()) log.Debugf("In artifact info middleware, url: %s", req.URL.String())
m, ok := parse(req.URL) m, ok, err := parse(req.URL)
if err != nil {
lib_http.SendError(rw, err)
return
}
if !ok { if !ok {
next.ServeHTTP(rw, req) next.ServeHTTP(rw, req)
return return
@ -100,7 +105,7 @@ func projectNameFromRepo(repo string) (string, error) {
return components[0], nil return components[0], nil
} }
func parse(url *url.URL) (map[string]string, bool) { func parse(url *url.URL) (map[string]string, bool, error) {
path := url.Path path := url.Path
query := url.Query() query := url.Query()
m := make(map[string]string) m := make(map[string]string)
@ -119,10 +124,15 @@ func parse(url *url.URL) (map[string]string, bool) {
break break
} }
} }
if digest.DigestRegexp.MatchString(m[lib.ReferenceSubexp]) { // parse reference, for invalid reference format, just give 404.
m[lib.DigestSubexp] = m[lib.ReferenceSubexp] if m[lib.ReferenceSubexp] != "" {
} else if ref, ok := m[lib.ReferenceSubexp]; ok { if digest.DigestRegexp.MatchString(m[lib.ReferenceSubexp]) {
m[tag] = ref m[lib.DigestSubexp] = m[lib.ReferenceSubexp]
} else if reference.TagRegexp.MatchString(m[lib.ReferenceSubexp]) {
m[tag] = m[lib.ReferenceSubexp]
} else {
return m, match, errors.New("invalid reference format").WithCode(errors.NotFoundCode)
}
} }
return m, match return m, match, nil
} }

View File

@ -16,6 +16,7 @@ package artifactinfo
import ( import (
"context" "context"
"github.com/goharbor/harbor/src/lib/errors"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"net/url" "net/url"
@ -30,6 +31,7 @@ func TestParseURL(t *testing.T) {
input string input string
expect map[string]string expect map[string]string
match bool match bool
rc string
}{ }{
{ {
input: "/api/projects", input: "/api/projects",
@ -60,8 +62,12 @@ func TestParseURL(t *testing.T) {
{ {
input: "/v2/development/golang/manifests/shaxxx:**********************************************************************************************************************************", input: "/v2/development/golang/manifests/shaxxx:**********************************************************************************************************************************",
expect: map[string]string{}, expect: map[string]string{
match: false, lib.RepositorySubexp: "development/golang",
lib.ReferenceSubexp: "shaxxx:**********************************************************************************************************************************",
"tag": "shaxxx:**********************************************************************************************************************************",
},
match: true,
}, },
{ {
input: "/v2/multi/sector/repository/blobs/sha256:08e4a417ff4e3913d8723a05cc34055db01c2fd165b588e049c5bad16ce6094f", input: "/v2/multi/sector/repository/blobs/sha256:08e4a417ff4e3913d8723a05cc34055db01c2fd165b588e049c5bad16ce6094f",
@ -99,6 +105,12 @@ func TestParseURL(t *testing.T) {
}, },
match: true, match: true,
}, },
{
input: "/v2/library/centos/manifest/.Invalid",
expect: map[string]string{},
match: false,
rc: errors.NotFoundCode,
},
} }
for _, c := range cases { for _, c := range cases {
@ -106,7 +118,10 @@ func TestParseURL(t *testing.T) {
if err != nil { if err != nil {
panic(err) panic(err)
} }
e, m := parse(url) e, m, err := parse(url)
if err != nil {
assert.True(t, errors.IsErr(err, c.rc))
}
assert.Equal(t, c.match, m) assert.Equal(t, c.match, m)
assert.Equal(t, c.expect, e) assert.Equal(t, c.expect, e)
} }