Merge pull request #11436 from ywk253100/200404_repo_encode

Update APIs to only accept encoded repository name that contains slash

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/portal/src/app/project/repository/artifact-list-page/artifact-list/artifact-list-tab/artifact-list-tab.component.ts

@@ -34,7 +34,7 @@ import {
   clone,
   CustomComparator,
   DEFAULT_PAGE_SIZE, DEFAULT_SUPPORTED_MIME_TYPE,
-  formatSize, VULNERABILITY_SCAN_STATUS
+  formatSize, VULNERABILITY_SCAN_STATUS, dbEncodeURIComponent
 } from "../../../../../../lib/utils/utils";
 import {
   ConfirmationAcknowledgement,
@@ -335,7 +335,7 @@ export class ArtifactListTabComponent implements OnInit, OnDestroy {
       }
       if (this.artifactDigest) {
         const artifactParam: NewArtifactService.GetArtifactParams = {
-          repositoryName: this.repoName,
+          repositoryName: dbEncodeURIComponent(this.repoName),
           projectName: this.projectName,
           reference: this.artifactDigest,
           withImmutableStatus: true,
@@ -351,7 +351,7 @@ export class ArtifactListTabComponent implements OnInit, OnDestroy {
             res.references.forEach((child, index) => {
               if (index >= (pageNumber - 1) * this.pageSize && index < pageNumber * this.pageSize) {
                 let childParams: NewArtifactService.GetArtifactParams = {
-                  repositoryName: this.repoName,
+                  repositoryName: dbEncodeURIComponent(this.repoName),
                   projectName: this.projectName,
                   reference: child.child_digest,
                   withImmutableStatus: true,
@@ -378,7 +378,7 @@ export class ArtifactListTabComponent implements OnInit, OnDestroy {
       } else {
         let listArtifactParams: NewArtifactService.ListArtifactsParams = {
           projectName: this.projectName,
-          repositoryName: this.repoName,
+          repositoryName: dbEncodeURIComponent(this.repoName),
           withLabel: true,
           withScanOverview: true,
           withTag: true
@@ -480,7 +480,7 @@ export class ArtifactListTabComponent implements OnInit, OnDestroy {
       this.selectedRow = this.selectedTag;
       let params: NewArtifactService.AddLabelParams = {
         projectName: this.projectName,
-        repositoryName: this.repoName,
+        repositoryName: dbEncodeURIComponent(this.repoName),
         reference: this.selectedRow[0].digest,
         label: labelInfo.label
       };
@@ -517,7 +517,7 @@ export class ArtifactListTabComponent implements OnInit, OnDestroy {
       this.selectedRow = this.selectedTag;
       let params: NewArtifactService.RemoveLabelParams = {
         projectName: this.projectName,
-        repositoryName: this.repoName,
+        repositoryName: dbEncodeURIComponent(this.repoName),
         reference: this.selectedRow[0].digest,
         labelId: labelId
       };
@@ -675,18 +675,18 @@ export class ArtifactListTabComponent implements OnInit, OnDestroy {
   onRetag() {
     let params: NewArtifactService.CopyArtifactParams = {
       projectName: this.imageNameInput.projectName.value,
-      repositoryName: this.imageNameInput.repoName.value,
+      repositoryName: dbEncodeURIComponent(this.imageNameInput.repoName.value),
       from: `${this.projectName}/${this.repoName}@${this.selectedRow[0].digest}`,
     };
     this.newArtifactService.CopyArtifact(params)
       .pipe(finalize(() => {
-        this.retagDialogOpened = false;
         this.imageNameInput.form.reset();
       }))
       .subscribe(response => {
         this.translateService.get('RETAG.MSG_SUCCESS').subscribe((res: string) => {
           this.errorHandlerService.info(res);
         });
+        this.retagDialogOpened = false;
       }, error => {
         this.errorHandlerService.error(error);
       });
@@ -779,7 +779,7 @@ export class ArtifactListTabComponent implements OnInit, OnDestroy {
     // } else {
       let params: NewArtifactService.DeleteArtifactParams = {
         projectName: this.projectName,
-        repositoryName: this.repoName,
+        repositoryName: dbEncodeURIComponent(this.repoName),
         reference: artifact.digest
       };
     return this.newArtifactService

src/portal/src/app/project/repository/artifact-list-page/artifact-list/artifact-list.component.ts

@@ -27,6 +27,7 @@ import { Project } from '../../../project';
 import {
   RepositoryService as NewRepositoryService
 } from "../../../../../../ng-swagger-gen/services/repository.service";
+import { dbEncodeURIComponent } from '../../../../../lib/utils/utils';
 const TabLinkContentMap: { [index: string]: string } = {
   'repo-info': 'info',
   'repo-image': 'image'
@@ -117,7 +118,7 @@ export class ArtifactListComponent implements OnInit {
   retrieve(state?: State) {
     let params: NewRepositoryService.GetRepositoryParams = {
       projectName: this.projectName,
-      repositoryName: this.repoName
+      repositoryName: dbEncodeURIComponent(this.repoName),
     };
     this.newRepositoryService.getRepository(params)
       .subscribe(response => {
@@ -168,7 +169,7 @@ export class ArtifactListComponent implements OnInit {
     }
     this.onGoing = true;
     let params: NewRepositoryService.UpdateRepositoryParams = {
-      repositoryName: this.repoName,
+      repositoryName: dbEncodeURIComponent(this.repoName),
       repository: {description: this.imageInfo},
       projectName: this.projectName,
     };

src/portal/src/app/project/repository/artifact/artifact-summary.component.ts

@@ -8,6 +8,7 @@ import { ActivatedRoute, Router } from "@angular/router";
 import { AppConfigService } from "../../../services/app-config.service";
 import { Project } from "../../project";
 import { finalize } from "rxjs/operators";
+import { dbEncodeURIComponent } from "../../../../lib/utils/utils";
 
 @Component({
   selector: "artifact-summary",
@@ -87,7 +88,7 @@ export class ArtifactSummaryComponent implements OnInit {
   getArtifactDetails(): void {
     this.loading = true;
     this.artifactService.getArtifact({
-      repositoryName: this.repositoryName,
+      repositoryName: dbEncodeURIComponent(this.repositoryName),
       reference: this.artifactDigest,
       projectName: this.projectName,
       withLabel: true,

src/portal/src/app/project/repository/artifact/artifact-tag/artifact-tag.component.ts

@@ -21,7 +21,7 @@ import {
   UserPermissionService, USERSTATICPERMISSION
 } from "../../../../../lib/services";
 import { ClrDatagridStateInterface } from '@clr/angular';
-import { DEFAULT_PAGE_SIZE, calculatePage } from '../../../../../lib/utils/utils';
+import { DEFAULT_PAGE_SIZE, calculatePage, dbEncodeURIComponent } from '../../../../../lib/utils/utils';
 
 class InitTag {
   name = "";
@@ -74,7 +74,7 @@ export class ArtifactTagComponent implements OnInit, OnDestroy {
   checkTagName(name) {
       let listArtifactParams: ArtifactService.ListArtifactsParams = {
         projectName: this.projectName,
-        repositoryName: this.repositoryName,
+        repositoryName: dbEncodeURIComponent(this.repositoryName),
         withLabel: true,
         withScanOverview: true,
         withTag: true,
@@ -111,7 +111,7 @@ export class ArtifactTagComponent implements OnInit, OnDestroy {
       if (pageNumber <= 0) { pageNumber = 1; }
     let params: ArtifactService.ListTagsParams = {
       projectName: this.projectName,
-      repositoryName: this.repositoryName,
+      repositoryName: dbEncodeURIComponent(this.repositoryName),
       reference: this.artifactDetails.digest,
       page: pageNumber,
       withSignature: true,
@@ -272,6 +272,8 @@ export class ArtifactTagComponent implements OnInit, OnDestroy {
   existValid(name) {
     if (name) {
       this.tagNameChecker.next(name);
+    } else {
+      this.isTagNameExist = false;
     }
   }
   toggleTagListOpenOrClose() {

src/portal/src/app/project/repository/repository-gridview.component.ts

@@ -24,7 +24,7 @@ import {
   UserPermissionService, USERSTATICPERMISSION
 } from "../../../lib/services";
 import { FilterComponent } from "../../../lib/components/filter/filter.component";
-import { calculatePage, clone, DEFAULT_PAGE_SIZE } from "../../../lib/utils/utils";
+import { calculatePage, clone, DEFAULT_PAGE_SIZE, dbEncodeURIComponent } from "../../../lib/utils/utils";
 import { IServiceConfig, SERVICE_CONFIG } from "../../../lib/entities/service.config";
 import { ErrorHandler } from "../../../lib/utils/error-handler";
 import { ConfirmationButtons, ConfirmationState, ConfirmationTargets } from "../../../lib/entities/shared.const";
@@ -222,7 +222,7 @@ export class RepositoryGridviewComponent implements OnChanges, OnInit, OnDestroy
     this.operationService.publishInfo(operMessage);
     return this.newRepoService
       .deleteRepository({
-        repositoryName: repo.name,
+        repositoryName: dbEncodeURIComponent(repo.name),
         projectName: this.projectName
       })
       .pipe(map(

src/portal/src/app/project/repository/vulnerability-scanning/result-bar-chart.component.ts

@@ -14,7 +14,8 @@ import {
     clone,
     CURRENT_BASE_HREF,
     DEFAULT_SUPPORTED_MIME_TYPE,
-    VULNERABILITY_SCAN_STATUS
+    VULNERABILITY_SCAN_STATUS,
+    dbEncodeURIComponent
 } from "../../../../lib/utils/utils";
 import { ArtifactService } from "../../../../../ng-swagger-gen/services/artifact.service";
 import { Artifact } from "../../../../../ng-swagger-gen/models/artifact";
@@ -118,7 +119,7 @@ export class ResultBarChartComponent implements OnInit, OnDestroy {
 
         this.onSubmitting = true;
 
-        this.scanningService.startVulnerabilityScanning(this.projectName, this.repoName, this.artifactDigest)
+        this.scanningService.startVulnerabilityScanning(this.projectName, dbEncodeURIComponent(this.repoName), this.artifactDigest)
             .pipe(finalize(() => this.submitFinish.emit(false)))
             .subscribe(() => {
                 this.onSubmitting = false;
@@ -149,7 +150,7 @@ export class ResultBarChartComponent implements OnInit, OnDestroy {
         }
         this.artifactService.getArtifact({
             projectName: this.projectName,
-            repositoryName: this.repoName,
+            repositoryName: dbEncodeURIComponent(this.repoName),
             reference: this.artifactDigest,
             withScanOverview: true
         })

src/portal/src/app/services/routing-resolvers/artifact-detail-routing-resolver.service.ts

@@ -19,6 +19,7 @@ import { Artifact } from "../../../../ng-swagger-gen/models/artifact";
 import { ArtifactService } from "../../../../ng-swagger-gen/services/artifact.service";
 import { Project } from "../../project/project";
 import { ProjectService } from "../../../lib/services";
+import { dbEncodeURIComponent } from '../../../lib/utils/utils';
 
 @Injectable({
   providedIn: 'root'
@@ -38,7 +39,7 @@ export class ArtifactDetailRoutingResolverService implements Resolve<Artifact> {
       .pipe(
           mergeMap((project: Project) => {
             return forkJoin([this.artifactService.getArtifact({
-              repositoryName: repositoryName,
+              repositoryName: dbEncodeURIComponent(repositoryName),
               reference: artifactDigest,
               projectName: project.name,
               withLabel: true,

src/portal/src/lib/utils/utils.ts

@@ -624,3 +624,9 @@ export function mergeDeep(target, ...sources) {
     }
     return mergeDeep(target, ...sources);
 }
+export function dbEncodeURIComponent(url: string) {
+    if (typeof url === "string") {
+        return encodeURIComponent(encodeURIComponent(url));
+    }
+    return "";
+}

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,

tests/apitests/python/test_project_quota.py

@@ -59,7 +59,7 @@ class TestProjects(unittest.TestCase):
                 self.assertEqual(quota[0].used["storage"], 2789002)
 
                 #8. Delete repository(RA) by user(UA);
-                self.repo.delete_repoitory(project_name, image, **ADMIN_CLIENT)
+                self.repo.delete_repoitory(project_name, "goharbor%2Falpine", **ADMIN_CLIENT)
 
                 #9. Quota should be 0
                 quota = self.system.get_project_quota("project", project_id, **ADMIN_CLIENT)