mirror of
https://github.com/project-zot/zot.git
synced 2026-06-15 11:37:56 +08:00
fix(api): return 416 for bad upload range on PUT; fix GET upload Range at size zero (#3983)
Align closing blob upload (PUT) with the OCI Distribution Spec: invalid / out-of-order upload ranges (ErrBadUploadRange) return 416 Requested Range Not Satisfiable instead of 400, for both the final-chunk PutBlobChunk path and FinishBlobUpload. GetBlobUpload (GET upload status): fix the Range response when zero bytes have been received—send Range: 0-0 instead of Range: 0--1, consistent with a new session and the spec’s Location + Range upload status shape. Only map ErrBadBlobDigest to 400 here; do not handle ErrBadUploadRange on GET (that request carries no range; ImageStore.GetBlobUpload does not return it). Document PUT upload failures 400 and 416 in swagger; regenerate swagger artifacts. Update route tests (expect 416 on UpdateBlobUpload for ErrBadUploadRange), drop the mock-only GetBlobUpload+ErrBadUploadRange case, and assert Range: 0-0 in TestPullRange after GET on a new upload location. Fix potential panic when parsing Content-Range (index out of range) when accessing `tokens[0]`. Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
This commit is contained in:
@@ -11223,6 +11223,7 @@ func TestPullRange(t *testing.T) {
|
||||
resp, err = resty.R().Get(loc)
|
||||
So(err, ShouldBeNil)
|
||||
So(resp.StatusCode(), ShouldEqual, http.StatusNoContent)
|
||||
So(resp.Header().Get("Range"), ShouldEqual, "0-0")
|
||||
|
||||
content := []byte("0123456789")
|
||||
digest := godigest.FromBytes(content)
|
||||
|
||||
+33
-9
@@ -1553,7 +1553,7 @@ func (rh *RouteHandler) GetBlobUpload(response http.ResponseWriter, request *htt
|
||||
if err != nil {
|
||||
details := zerr.GetDetails(err)
|
||||
//nolint:gocritic // errorslint conflicts with gocritic:IfElseChain
|
||||
if errors.Is(err, zerr.ErrBadUploadRange) || errors.Is(err, zerr.ErrBadBlobDigest) {
|
||||
if errors.Is(err, zerr.ErrBadBlobDigest) {
|
||||
details["session_id"] = sessionID
|
||||
e := apiErr.NewError(apiErr.BLOB_UPLOAD_INVALID).AddDetail(details)
|
||||
zcommon.WriteJSON(response, http.StatusBadRequest, apiErr.NewErrorList(e))
|
||||
@@ -1574,7 +1574,13 @@ func (rh *RouteHandler) GetBlobUpload(response http.ResponseWriter, request *htt
|
||||
}
|
||||
|
||||
response.Header().Set("Location", getBlobUploadSessionLocation(request.URL, sessionID))
|
||||
response.Header().Set("Range", fmt.Sprintf("0-%d", size-1))
|
||||
// Match POST new-upload Range for empty progress; otherwise 0..size-1 per dist-spec upload status.
|
||||
rangeEnd := "0-0"
|
||||
if size > 0 {
|
||||
rangeEnd = fmt.Sprintf("0-%d", size-1)
|
||||
}
|
||||
|
||||
response.Header().Set("Range", rangeEnd)
|
||||
response.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
|
||||
@@ -1688,7 +1694,9 @@ func (rh *RouteHandler) PatchBlobUpload(response http.ResponseWriter, request *h
|
||||
// @Success 201 "created"
|
||||
// @Header 201 {string} Location "/v2/{name}/blobs/{digest}"
|
||||
// @Header 201 {string} Docker-Content-Digest "Digest of the committed blob"
|
||||
// @Failure 400 {string} string "bad request"
|
||||
// @Failure 404 {string} string "not found"
|
||||
// @Failure 416 {string} string "range not satisfiable"
|
||||
// @Failure 500 {string} string "internal server error"
|
||||
// @Router /v2/{name}/blobs/uploads/{session_id} [put].
|
||||
func (rh *RouteHandler) UpdateBlobUpload(response http.ResponseWriter, request *http.Request) {
|
||||
@@ -1758,7 +1766,10 @@ func (rh *RouteHandler) UpdateBlobUpload(response http.ResponseWriter, request *
|
||||
|
||||
to = contentLen
|
||||
} else if from, to, err = getContentRange(request); err != nil { // finish chunked upload
|
||||
response.WriteHeader(http.StatusRequestedRangeNotSatisfiable)
|
||||
details := zerr.GetDetails(err)
|
||||
details["session_id"] = sessionID
|
||||
e := apiErr.NewError(apiErr.BLOB_UPLOAD_INVALID).AddDetail(details)
|
||||
zcommon.WriteJSON(response, http.StatusRequestedRangeNotSatisfiable, apiErr.NewErrorList(e))
|
||||
|
||||
return
|
||||
}
|
||||
@@ -1769,7 +1780,7 @@ func (rh *RouteHandler) UpdateBlobUpload(response http.ResponseWriter, request *
|
||||
if errors.Is(err, zerr.ErrBadUploadRange) { //nolint:gocritic // errorslint conflicts with gocritic:IfElseChain
|
||||
details["session_id"] = sessionID
|
||||
e := apiErr.NewError(apiErr.BLOB_UPLOAD_INVALID).AddDetail(details)
|
||||
zcommon.WriteJSON(response, http.StatusBadRequest, apiErr.NewErrorList(e))
|
||||
zcommon.WriteJSON(response, http.StatusRequestedRangeNotSatisfiable, apiErr.NewErrorList(e))
|
||||
} else if errors.Is(err, zerr.ErrRepoNotFound) {
|
||||
details["name"] = name
|
||||
e := apiErr.NewError(apiErr.NAME_UNKNOWN).AddDetail(details)
|
||||
@@ -1805,7 +1816,7 @@ finish:
|
||||
} else if errors.Is(err, zerr.ErrBadUploadRange) {
|
||||
details["session_id"] = sessionID
|
||||
e := apiErr.NewError(apiErr.BLOB_UPLOAD_INVALID).AddDetail(details)
|
||||
zcommon.WriteJSON(response, http.StatusBadRequest, apiErr.NewErrorList(e))
|
||||
zcommon.WriteJSON(response, http.StatusRequestedRangeNotSatisfiable, apiErr.NewErrorList(e))
|
||||
} else if errors.Is(err, zerr.ErrRepoNotFound) {
|
||||
details["name"] = name
|
||||
e := apiErr.NewError(apiErr.NAME_UNKNOWN).AddDetail(details)
|
||||
@@ -2216,15 +2227,28 @@ func (rh *RouteHandler) OpenIDCodeExchangeCallbackWithProvider(providerName stri
|
||||
// helper routines
|
||||
|
||||
func getContentRange(r *http.Request) (int64 /* from */, int64 /* to */, error) {
|
||||
contentRange := r.Header.Get("Content-Range")
|
||||
tokens := strings.Split(contentRange, "-")
|
||||
contentRange := strings.TrimSpace(r.Header.Get("Content-Range"))
|
||||
if contentRange == "" {
|
||||
return -1, -1, zerr.ErrBadUploadRange
|
||||
}
|
||||
|
||||
rangeStart, err := strconv.ParseInt(tokens[0], 10, 64)
|
||||
startStr, endStr, ok := strings.Cut(contentRange, "-")
|
||||
if !ok {
|
||||
return -1, -1, zerr.ErrBadUploadRange
|
||||
}
|
||||
|
||||
startStr = strings.TrimSpace(startStr)
|
||||
endStr = strings.TrimSpace(endStr)
|
||||
if startStr == "" || endStr == "" {
|
||||
return -1, -1, zerr.ErrBadUploadRange
|
||||
}
|
||||
|
||||
rangeStart, err := strconv.ParseInt(startStr, 10, 64)
|
||||
if err != nil {
|
||||
return -1, -1, zerr.ErrBadUploadRange
|
||||
}
|
||||
|
||||
rangeEnd, err := strconv.ParseInt(tokens[1], 10, 64)
|
||||
rangeEnd, err := strconv.ParseInt(endStr, 10, 64)
|
||||
if err != nil {
|
||||
return -1, -1, zerr.ErrBadUploadRange
|
||||
}
|
||||
|
||||
+20
-18
@@ -929,23 +929,8 @@ func TestRoutes(t *testing.T) {
|
||||
return resp.StatusCode
|
||||
}
|
||||
|
||||
// ErrBadUploadRange
|
||||
statusCode := testGetBlobUpload(
|
||||
[]struct{ k, v string }{},
|
||||
map[string]string{},
|
||||
map[string]string{
|
||||
"name": "test",
|
||||
"session_id": "1234",
|
||||
},
|
||||
&mocks.MockedImageStore{
|
||||
GetBlobUploadFn: func(repo, uuid string) (int64, error) {
|
||||
return 0, zerr.ErrBadUploadRange
|
||||
},
|
||||
})
|
||||
So(statusCode, ShouldEqual, http.StatusBadRequest)
|
||||
|
||||
// ErrBadBlobDigest
|
||||
statusCode = testGetBlobUpload(
|
||||
statusCode := testGetBlobUpload(
|
||||
[]struct{ k, v string }{
|
||||
{"mount", "1234"},
|
||||
},
|
||||
@@ -1195,6 +1180,23 @@ func TestRoutes(t *testing.T) {
|
||||
)
|
||||
So(status, ShouldEqual, http.StatusRequestedRangeNotSatisfiable)
|
||||
|
||||
// Malformed Content-Range (no hyphen): must return 416, not panic.
|
||||
status = testUpdateBlobUpload(
|
||||
[]struct{ k, v string }{
|
||||
{"digest", "sha256:7b8437f04f83f084b7ed68ad8c4a4947e12fc4e1b006b38129bac89114ec3621"},
|
||||
},
|
||||
map[string]string{
|
||||
"Content-Length": "100",
|
||||
"Content-Range": "100",
|
||||
},
|
||||
map[string]string{
|
||||
"name": "repo",
|
||||
"session_id": "test",
|
||||
},
|
||||
&mocks.MockedImageStore{},
|
||||
)
|
||||
So(status, ShouldEqual, http.StatusRequestedRangeNotSatisfiable)
|
||||
|
||||
status = testUpdateBlobUpload(
|
||||
[]struct{ k, v string }{
|
||||
{"digest", "sha256:7b8437f04f83f084b7ed68ad8c4a4947e12fc4e1b006b38129bac89114ec3621"},
|
||||
@@ -1213,7 +1215,7 @@ func TestRoutes(t *testing.T) {
|
||||
},
|
||||
},
|
||||
)
|
||||
So(status, ShouldEqual, http.StatusBadRequest)
|
||||
So(status, ShouldEqual, http.StatusRequestedRangeNotSatisfiable)
|
||||
|
||||
status = testUpdateBlobUpload(
|
||||
[]struct{ k, v string }{
|
||||
@@ -1316,7 +1318,7 @@ func TestRoutes(t *testing.T) {
|
||||
},
|
||||
},
|
||||
)
|
||||
So(status, ShouldEqual, http.StatusBadRequest)
|
||||
So(status, ShouldEqual, http.StatusRequestedRangeNotSatisfiable)
|
||||
|
||||
status = testUpdateBlobUpload(
|
||||
[]struct{ k, v string }{
|
||||
|
||||
@@ -438,12 +438,24 @@ const docTemplate = `{
|
||||
}
|
||||
}
|
||||
},
|
||||
"400": {
|
||||
"description": "bad request",
|
||||
"schema": {
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"404": {
|
||||
"description": "not found",
|
||||
"schema": {
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"416": {
|
||||
"description": "range not satisfiable",
|
||||
"schema": {
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"500": {
|
||||
"description": "internal server error",
|
||||
"schema": {
|
||||
|
||||
@@ -430,12 +430,24 @@
|
||||
}
|
||||
}
|
||||
},
|
||||
"400": {
|
||||
"description": "bad request",
|
||||
"schema": {
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"404": {
|
||||
"description": "not found",
|
||||
"schema": {
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"416": {
|
||||
"description": "range not satisfiable",
|
||||
"schema": {
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"500": {
|
||||
"description": "internal server error",
|
||||
"schema": {
|
||||
|
||||
@@ -674,10 +674,18 @@ paths:
|
||||
Location:
|
||||
description: /v2/{name}/blobs/{digest}
|
||||
type: string
|
||||
"400":
|
||||
description: bad request
|
||||
schema:
|
||||
type: string
|
||||
"404":
|
||||
description: not found
|
||||
schema:
|
||||
type: string
|
||||
"416":
|
||||
description: range not satisfiable
|
||||
schema:
|
||||
type: string
|
||||
"500":
|
||||
description: internal server error
|
||||
schema:
|
||||
|
||||
Reference in New Issue
Block a user