From 620bc7c517dc9c5c41e918f7c6bf8026633fd6cf Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 2 Jun 2022 20:54:42 +0000 Subject: [PATCH] routes: strip query parameter from request URL reuqest url also contains query parameter due to this in some scenarios location header is setting up incorrectly, strip query parameter from request url to correctly setup location header. Closes #573 #575 Signed-off-by: Shivam Mishra --- pkg/api/constants/consts.go | 2 ++ pkg/api/controller_test.go | 12 +++++++++++ pkg/api/routes.go | 43 +++++++++++++++++++++++++++++++------ pkg/test/common.go | 3 --- 4 files changed, 50 insertions(+), 10 deletions(-) diff --git a/pkg/api/constants/consts.go b/pkg/api/constants/consts.go index 05ac0ec3..057ad343 100644 --- a/pkg/api/constants/consts.go +++ b/pkg/api/constants/consts.go @@ -3,6 +3,8 @@ package constants const ( ArtifactSpecRoutePrefix = "/oras/artifacts/v1" RoutePrefix = "/v2" + Blobs = "blobs" + Uploads = "uploads" DistAPIVersion = "Docker-Distribution-API-Version" DistContentDigestKey = "Docker-Content-Digest" BlobUploadUUID = "Blob-Upload-UUID" diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index d4960f51..da11ac75 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -2719,6 +2719,10 @@ func TestCrossRepoMount(t *testing.T) { Post(baseURL + "/v2/zot-c-test/blobs/uploads/") So(err, ShouldBeNil) So(postResponse.StatusCode(), ShouldEqual, http.StatusAccepted) + location, err := postResponse.RawResponse.Location() + So(err, ShouldBeNil) + So(location.String(), ShouldStartWith, fmt.Sprintf("%s%s/zot-c-test/%s/%s", + baseURL, constants.RoutePrefix, constants.Blobs, constants.Uploads)) incorrectParams := make(map[string]string) incorrectParams["mount"] = "sha256:63a795ca90aa6e7dda60941e826810a4cd0a2e73ea02bf458241df2a5c973e29" @@ -2729,6 +2733,8 @@ func TestCrossRepoMount(t *testing.T) { Post(baseURL + "/v2/zot-y-test/blobs/uploads/") So(err, ShouldBeNil) So(postResponse.StatusCode(), ShouldEqual, http.StatusAccepted) + So(test.Location(baseURL, postResponse), ShouldStartWith, fmt.Sprintf("%s%s/zot-y-test/%s/%s", + baseURL, constants.RoutePrefix, constants.Blobs, constants.Uploads)) // Use correct request // This is correct request but it will return 202 because blob is not present in cache. @@ -2738,6 +2744,8 @@ func TestCrossRepoMount(t *testing.T) { Post(baseURL + "/v2/zot-c-test/blobs/uploads/") So(err, ShouldBeNil) So(postResponse.StatusCode(), ShouldEqual, http.StatusAccepted) + So(test.Location(baseURL, postResponse), ShouldStartWith, fmt.Sprintf("%s%s/zot-c-test/%s/%s", + baseURL, constants.RoutePrefix, constants.Blobs, constants.Uploads)) // Send same request again postResponse, err = client.R(). @@ -2792,6 +2800,8 @@ func TestCrossRepoMount(t *testing.T) { Post(baseURL + "/v2/zot-mount-test/blobs/uploads/") So(err, ShouldBeNil) So(postResponse.StatusCode(), ShouldEqual, http.StatusCreated) + So(test.Location(baseURL, postResponse), ShouldEqual, fmt.Sprintf("%s%s/zot-mount-test/%s/%s:%s", + baseURL, constants.RoutePrefix, constants.Blobs, godigest.SHA256, blob)) // Check os.SameFile here cachePath := path.Join(ctlr.Config.Storage.RootDirectory, "zot-d-test", "blobs/sha256", dgst.Hex()) @@ -2815,6 +2825,8 @@ func TestCrossRepoMount(t *testing.T) { Post(baseURL + "/v2/zot-mount1-test/blobs/uploads/") So(err, ShouldBeNil) So(postResponse.StatusCode(), ShouldEqual, http.StatusCreated) + So(test.Location(baseURL, postResponse), ShouldEqual, fmt.Sprintf("%s%s/zot-mount1-test/%s/%s:%s", + baseURL, constants.RoutePrefix, constants.Blobs, godigest.SHA256, blob)) linkPath = path.Join(ctlr.Config.Storage.RootDirectory, "zot-mount1-test", "blobs/sha256", dgst.Hex()) diff --git a/pkg/api/routes.go b/pkg/api/routes.go index bf13d90f..317f9407 100644 --- a/pkg/api/routes.go +++ b/pkg/api/routes.go @@ -17,6 +17,7 @@ import ( "io" "io/ioutil" "net/http" + "net/url" "path" "sort" "strconv" @@ -759,14 +760,14 @@ func (rh *RouteHandler) CreateBlobUpload(response http.ResponseWriter, request * return } - response.Header().Set("Location", path.Join(request.URL.String(), upload)) + response.Header().Set("Location", getBlobUploadSessionLocation(request.URL, upload)) response.Header().Set("Range", "bytes=0-0") response.WriteHeader(http.StatusAccepted) return } - response.Header().Set("Location", fmt.Sprintf("/v2/%s/blobs/%s", name, mountDigests[0])) + response.Header().Set("Location", getBlobUploadLocation(request.URL, name, mountDigests[0])) response.WriteHeader(http.StatusCreated) return @@ -826,7 +827,7 @@ func (rh *RouteHandler) CreateBlobUpload(response http.ResponseWriter, request * return } - response.Header().Set("Location", fmt.Sprintf("/v2/%s/blobs/%s", name, digest)) + response.Header().Set("Location", getBlobUploadLocation(request.URL, name, digest)) response.Header().Set(constants.BlobUploadUUID, sessionID) response.WriteHeader(http.StatusCreated) @@ -845,7 +846,7 @@ func (rh *RouteHandler) CreateBlobUpload(response http.ResponseWriter, request * return } - response.Header().Set("Location", path.Join(request.URL.String(), upload)) + response.Header().Set("Location", getBlobUploadSessionLocation(request.URL, upload)) response.Header().Set("Range", "bytes=0-0") response.WriteHeader(http.StatusAccepted) } @@ -904,7 +905,7 @@ func (rh *RouteHandler) GetBlobUpload(response http.ResponseWriter, request *htt return } - response.Header().Set("Location", path.Join(request.URL.String(), sessionID)) + response.Header().Set("Location", getBlobUploadSessionLocation(request.URL, sessionID)) response.Header().Set("Range", fmt.Sprintf("bytes=0-%d", size-1)) response.WriteHeader(http.StatusNoContent) } @@ -996,7 +997,7 @@ func (rh *RouteHandler) PatchBlobUpload(response http.ResponseWriter, request *h return } - response.Header().Set("Location", request.URL.String()) + response.Header().Set("Location", getBlobUploadSessionLocation(request.URL, sessionID)) response.Header().Set("Range", fmt.Sprintf("bytes=0-%d", clen-1)) response.Header().Set("Content-Length", "0") response.Header().Set(constants.BlobUploadUUID, sessionID) @@ -1139,7 +1140,7 @@ finish: return } - response.Header().Set("Location", fmt.Sprintf("/v2/%s/blobs/%s", name, digest)) + response.Header().Set("Location", getBlobUploadLocation(request.URL, name, digest)) response.Header().Set("Content-Length", "0") response.Header().Set(constants.DistContentDigestKey, digest) response.WriteHeader(http.StatusCreated) @@ -1465,3 +1466,31 @@ func (rh *RouteHandler) GetReferrers(response http.ResponseWriter, request *http WriteJSON(response, http.StatusOK, rs) } + +// GetBlobUploadSessionLocation returns actual blob location to start/resume uploading blobs. +// e.g. /v2//blobs/uploads/. +func getBlobUploadSessionLocation(url *url.URL, sessionID string) string { + url.RawQuery = "" + + if !strings.Contains(url.Path, sessionID) { + url.Path = path.Join(url.Path, sessionID) + } + + return url.String() +} + +// GetBlobUploadLocation returns actual blob location on registry +// e.g /v2//blobs/. +func getBlobUploadLocation(url *url.URL, name, digest string) string { + url.RawQuery = "" + + // we are relying on request URL to set location and + // if request URL contains uploads either we are resuming blob upload or starting a new blob upload. + // getBlobUploadLocation will be called only when blob upload is completed and + // location should be set as blob url /blobs/>. + if strings.Contains(url.Path, "uploads") { + url.Path = path.Join(constants.RoutePrefix, name, constants.Blobs, digest) + } + + return url.String() +} diff --git a/pkg/test/common.go b/pkg/test/common.go index cd4e2dde..ac8b29d8 100644 --- a/pkg/test/common.go +++ b/pkg/test/common.go @@ -80,9 +80,6 @@ func Location(baseURL string, resp *resty.Response) string { } path := uloc.Path - if query := uloc.RawQuery; query != "" { - path += "?" + query - } return baseURL + path }