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 <[email protected]>
project-zot · Jun 9, 2022 · 620bc7c · 620bc7c
1 parent f52c950
commit 620bc7c
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 10 deletions.
diff --git 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
@@ -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
@@ -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/<name>/blobs/uploads/<session-id>.
+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/<name>/blobs/<digest>.
+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 <v2/<name>/blobs/<digest>>.
+	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
@@ -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
 }