Update APIs to only accept encoded repository name that contains slash

Update APIs to only accept encoded repository name that contains slash

Signed-off-by: Wenkai Yin <yinw@vmware.com>

api/v2.0/swagger.yaml

@@ -53,11 +53,7 @@ paths:
           $ref: '#/responses/404'
         '500':
           $ref: '#/responses/500'
-  # the _self suffix here is used to avoid the conflict of repository name and URL path
+  /projects/{project_name}/repositories/{repository_name}:
-  # e.g. the repository name can be "library/artifacts", we cannot distinguish the URL
-  # "GET /projects/{project_name}/repositories/library/artifacts" is getting repository
-  # or listing artifacts
-  /projects/{project_name}/repositories/{repository_name}/_self:
     get:
       summary: Get repository
       description: Get the repository specified by name
@@ -671,7 +667,7 @@ parameters:
   repositoryName:
     name: repository_name
     in: path
-    description: The name of the repository
+    description: The name of the repository. If it contains slash, encode it with URL encoding. e.g. a/b -> a%252Fb
     required: true
     type: string
   reference:

src/controller/artifact/model.go

@@ -16,12 +16,13 @@ package artifact
 
 import (
 	"fmt"
-	"github.com/goharbor/harbor/src/server/v2.0/models"
+	"net/url"
 
 	cmodels "github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/utils"
 	"github.com/goharbor/harbor/src/controller/tag"
 	"github.com/goharbor/harbor/src/pkg/artifact"
+	"github.com/goharbor/harbor/src/server/v2.0/models"
 )
 
 // Artifact is the overall view of artifact
@@ -39,6 +40,8 @@ func (artifact *Artifact) SetAdditionLink(addition, version string) {
 	}
 
 	projectName, repo := utils.ParseRepository(artifact.RepositoryName)
+	// encode slash as %252F
+	repo = url.PathEscape(url.PathEscape(repo))
 	href := fmt.Sprintf("/api/%s/projects/%s/repositories/%s/artifacts/%s/additions/%s", version, projectName, repo, artifact.Digest, addition)
 
 	artifact.AdditionLinks[addition] = &AdditionLink{HREF: href, Absolute: false}

src/server/middleware/path/path.go

@@ -1,100 +0,0 @@
-// Copyright Project Harbor Authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package path
-
-import (
-	"net/http"
-	"net/url"
-	"regexp"
-
-	"github.com/goharbor/harbor/src/common/api"
-	"github.com/goharbor/harbor/src/server/middleware"
-)
-
-var (
-	defaultRegexps = []*regexp.Regexp{
-		regexp.MustCompile(`^/api/` + api.APIVersion + `/projects/.*/repositories/(.*)/_self/?$`),
-		regexp.MustCompile(`^/api/` + api.APIVersion + `/projects/.*/repositories/(.*)/artifacts/?$`),
-		regexp.MustCompile(`^/api/` + api.APIVersion + `/projects/.*/repositories/(.*)/artifacts/.*$`),
-	}
-)
-
-// EscapeMiddleware middleware which escape path parameters for swagger APIs
-func EscapeMiddleware() func(http.Handler) http.Handler {
-	return middleware.New(func(w http.ResponseWriter, r *http.Request, next http.Handler) {
-		for _, re := range defaultRegexps {
-			if re.MatchString(r.URL.Path) {
-				r.URL.Path = escape(re, r.URL.Path)
-				break
-			}
-		}
-
-		next.ServeHTTP(w, r)
-	})
-}
-
-func escape(re *regexp.Regexp, path string) string {
-	return replaceAllSubmatchFunc(re, path, func(groups []string) []string {
-		var results []string
-		for _, group := range groups {
-			results = append(results, url.PathEscape(group))
-		}
-		return results
-	}, -1)
-}
-
-func replaceAllSubmatchFunc(re *regexp.Regexp, src string, repl func([]string) []string, n int) string {
-	var result string
-
-	last := 0
-	for _, match := range re.FindAllSubmatchIndex([]byte(src), n) {
-		// Append string between our last match and this one (i.e. non-matched string).
-		matchStart := match[0]
-		matchEnd := match[1]
-		result = result + src[last:matchStart]
-		last = matchEnd
-
-		// Determine the groups / submatch string and indices.
-		groups := []string{}
-		indices := [][2]int{}
-		for i := 2; i < len(match); i += 2 {
-			start := match[i]
-			end := match[i+1]
-			groups = append(groups, src[start:end])
-			indices = append(indices, [2]int{start, end})
-		}
-
-		// Replace the groups
-		groups = repl(groups)
-
-		// Append match data.
-		lastGroup := matchStart
-		for i, newValue := range groups {
-			// Append string between our last group match and this one (i.e. non-group-matched string)
-			groupStart := indices[i][0]
-			groupEnd := indices[i][1]
-			result = result + src[lastGroup:groupStart]
-			lastGroup = groupEnd
-
-			// Append the new group value.
-			result = result + newValue
-		}
-		result = result + src[lastGroup:matchEnd] // remaining
-	}
-
-	result = result + src[last:] // remaining
-
-	return result
-}

src/server/middleware/path/path_test.go

@@ -1,84 +0,0 @@
-// Copyright Project Harbor Authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package path
-
-import (
-	"net/http"
-	"net/http/httptest"
-	"regexp"
-	"testing"
-)
-
-func Test_escape(t *testing.T) {
-	re := regexp.MustCompile(`/api/v2.0/projects/.*/repositories/(.*)/artifacts`)
-
-	type args struct {
-		re   *regexp.Regexp
-		path string
-	}
-	tests := []struct {
-		name string
-		args args
-		want string
-	}{
-		{
-			"/api/v2.0/projects/library/repositories/photon/artifacts",
-			args{re, "/api/v2.0/projects/library/repositories/photon/artifacts"},
-			"/api/v2.0/projects/library/repositories/photon/artifacts",
-		},
-		{
-			"/api/v2.0/projects/library/repositories/photon/hello-world/artifacts",
-			args{re, "/api/v2.0/projects/library/repositories/photon/hello-world/artifacts"},
-			"/api/v2.0/projects/library/repositories/photon%2Fhello-world/artifacts",
-		},
-		{
-			"/api/v2.0/projects/library/repositories/photon/hello-world/artifacts/digest/scan",
-			args{re, "/api/v2.0/projects/library/repositories/photon/hello-world/artifacts/digest/scan"},
-			"/api/v2.0/projects/library/repositories/photon%2Fhello-world/artifacts/digest/scan",
-		},
-
-		{
-			"/api/v2.0/projects/library/repositories",
-			args{re, "/api/v2.0/projects/library/repositories"},
-			"/api/v2.0/projects/library/repositories",
-		},
-		{
-			"/api/v2.0/projects/library/repositories/hello/mariadb/_self",
-			args{regexp.MustCompile(`^/api/v2.0/projects/.*/repositories/(.*)/_self`), "/api/v2.0/projects/library/repositories/hello/mariadb/_self"},
-			"/api/v2.0/projects/library/repositories/hello%2Fmariadb/_self",
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := escape(tt.args.re, tt.args.path); got != tt.want {
-				t.Errorf("escape() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
-func TestEscapeMiddleware(t *testing.T) {
-	r := httptest.NewRequest(http.MethodGet, "/api/v2.0/projects/library/repositories/hello/mariadb/_self", nil)
-	w := httptest.NewRecorder()
-
-	next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if r.URL.Path != "/api/v2.0/projects/library/repositories/hello%2Fmariadb/_self" {
-			t.Errorf("escape middleware failed")
-		}
-		w.WriteHeader(http.StatusOK)
-	})
-
-	EscapeMiddleware()(next).ServeHTTP(w, r)
-}

src/server/v2.0/handler/base.go

@@ -139,12 +139,6 @@ func (b *BaseAPI) Links(ctx context.Context, u *url.URL, total, pageNumber, page
 		return links
 	}
 	ul := *u
-	// try to unescape the repository name which contains escaped slashes
-	if escapedPath, err := url.PathUnescape(ul.Path); err == nil {
-		ul.Path = escapedPath
-	} else {
-		log.Errorf("failed to unescape the path %s: %v", ul.Path, err)
-	}
 	// prev
 	if pageNumber > 1 && (pageNumber-1)*pageSize < total {
 		q := ul.Query()

src/server/v2.0/handler/base_test.go

@@ -100,9 +100,9 @@ func (b *baseHandlerTestSuite) TestLinks() {
 	links = b.base.Links(nil, url, 3, 2, 1)
 	b.Require().Len(links, 2)
 	b.Equal("prev", links[0].Rel)
-	b.Equal("http://localhost/api/library/hello-world/artifacts?page=1&page_size=1&q=a=~b", links[0].URL)
+	b.Equal("http://localhost/api/library%252Fhello-world/artifacts?page=1&page_size=1&q=a=~b", links[0].URL)
 	b.Equal("next", links[1].Rel)
-	b.Equal("http://localhost/api/library/hello-world/artifacts?page=3&page_size=1&q=a=~b", links[1].URL)
+	b.Equal("http://localhost/api/library%252Fhello-world/artifacts?page=3&page_size=1&q=a=~b", links[1].URL)
 }
 
 func TestBaseHandler(t *testing.T) {

src/server/v2.0/handler/handler.go

@@ -21,7 +21,6 @@ import (
 	serror "github.com/goharbor/harbor/src/server/error"
 	"github.com/goharbor/harbor/src/server/middleware"
 	"github.com/goharbor/harbor/src/server/middleware/blob"
-	"github.com/goharbor/harbor/src/server/middleware/path"
 	"github.com/goharbor/harbor/src/server/middleware/quota"
 	"github.com/goharbor/harbor/src/server/v2.0/restapi"
 )
@@ -45,9 +44,7 @@ func New() http.Handler {
 
 	api.ServeError = serveError
 
-	// HACK: Use path.EscapeMiddleware to escape same patterns of the URL before the swagger handler
+	return h
-	// eg /api/v2.0/projects/library/repositories/hello/world/artifacts to /api/v2.0/projects/library/repositories/hello%2Fworld/artifacts
-	return path.EscapeMiddleware()(h)
 }
 
 // Before executing operation handler, go-swagger will bind a parameters object to a request and validate the request,