mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2025-06-17 11:59:30 +00:00
bd6f3243ab
# Fix OCI artifact uploads with`oras` ## Problem ORAS (OCI Registry As Storage) artifact uploads were failing with several HTTP-related errors when pushing to Forgejo's container registry. This prevented users from storing OCI artifacts like `artifacthub-repo.yaml` in commands like `oras push [...] artifacthub-repo.yaml:application/vnd.cncf.artifacthub.repository-metadata.layer.v1.yaml`. This has been discussed previously in https://github.com/go-gitea/gitea/issues/25846 ## Root Causes and Fixes ### 1. Missing Content-Length for Empty Blobs **Issue**: Empty blobs (size 0) were not getting the required `Content-Length: 0` header, causing ORAS to fail with "unknown response Content-Length". **Fix**: Changed the condition in `setResponseHeaders` from `if h.ContentLength != 0` to `if h.ContentLength >= 0` to ensure the Content-Length header is always set for valid blob sizes. ```go // Before if h.ContentLength != 0 { resp.Header().Set("Content-Length", strconv.FormatInt(h.ContentLength, 10)) } // After if h.ContentLength >= 0 { resp.Header().Set("Content-Length", strconv.FormatInt(h.ContentLength, 10)) } ``` ### 2. Content-Length Mismatch in JSON Error Responses **Issue**: The `jsonResponse` function was calling `WriteHeader()` before writing JSON content, causing "wrote more than the declared Content-Length" errors when the HTTP stack calculated a different Content-Length than what was actually written. **Fix**: Modified `jsonResponse` to buffer JSON content first, calculate the exact Content-Length, then write the complete response. ### 3. Incomplete HTTP Responses in Error Handling **Issue**: The `apiError` function was only setting response headers without writing any response body, causing EOF errors when clients expected a complete HTTP response. **Fix**: Updated `apiError` to write proper JSON error responses following the OCI Distribution Specification format with `code` and `message` fields. ### 4. Empty Config Blob Handling for OCI Artifacts **Issue**: OCI artifacts often have empty config blobs (required by spec but contain no data). The JSON decoder was failing with EOF when trying to parse these empty configs. **Fix**: Added EOF handling in `parseOCIImageConfig` to return a valid default metadata object for empty config blobs. ```go if err := json.NewDecoder(r).Decode(&image); err != nil { // Handle empty config blobs (common in OCI artifacts) if err == io.EOF { return &Metadata{ Type: TypeOCI, Platform: DefaultPlatform, }, nil } return nil, err } ``` ## Testing Verified that ORAS artifact uploads now work correctly: ```bash oras push registry/owner/package:artifacthub.io \ --config /dev/null:application/vnd.cncf.artifacthub.config.v1+yaml \ artifacthub-repo.yaml:application/vnd.cncf.artifacthub.repository-metadata.layer.v1.yaml ``` ### Tests - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [x] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8070 Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org> Co-authored-by: pat-s <patrick.schratz@gmail.com> Co-committed-by: pat-s <patrick.schratz@gmail.com>
124 lines
5 KiB
Go
124 lines
5 KiB
Go
// Copyright 2025 The Forgejo Authors. All rights reserved.
|
|
// SPDX-License-Identifier: MIT
|
|
|
|
package container
|
|
|
|
import (
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
)
|
|
|
|
func TestSetResponseHeaders(t *testing.T) {
|
|
t.Run("Content-Length for empty content", func(t *testing.T) {
|
|
recorder := httptest.NewRecorder()
|
|
|
|
setResponseHeaders(recorder, &containerHeaders{
|
|
Status: http.StatusOK,
|
|
ContentLength: 0, // Empty blob
|
|
ContentDigest: "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
|
|
})
|
|
|
|
assert.Equal(t, "0", recorder.Header().Get("Content-Length"))
|
|
assert.Equal(t, "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", recorder.Header().Get("Docker-Content-Digest"))
|
|
assert.Equal(t, "registry/2.0", recorder.Header().Get("Docker-Distribution-Api-Version"))
|
|
assert.Equal(t, http.StatusOK, recorder.Code)
|
|
})
|
|
|
|
t.Run("Content-Length for non-empty content", func(t *testing.T) {
|
|
recorder := httptest.NewRecorder()
|
|
|
|
setResponseHeaders(recorder, &containerHeaders{
|
|
Status: http.StatusOK,
|
|
ContentLength: 1024,
|
|
ContentDigest: "sha256:abcd1234",
|
|
})
|
|
|
|
assert.Equal(t, "1024", recorder.Header().Get("Content-Length"))
|
|
assert.Equal(t, "sha256:abcd1234", recorder.Header().Get("Docker-Content-Digest"))
|
|
})
|
|
|
|
t.Run("All headers set correctly", func(t *testing.T) {
|
|
recorder := httptest.NewRecorder()
|
|
|
|
setResponseHeaders(recorder, &containerHeaders{
|
|
Status: http.StatusAccepted,
|
|
ContentLength: 512,
|
|
ContentDigest: "sha256:test123",
|
|
ContentType: "application/vnd.oci.image.manifest.v1+json",
|
|
Location: "/v2/test/repo/blobs/uploads/uuid123",
|
|
Range: "0-511",
|
|
UploadUUID: "uuid123",
|
|
})
|
|
|
|
assert.Equal(t, "512", recorder.Header().Get("Content-Length"))
|
|
assert.Equal(t, "sha256:test123", recorder.Header().Get("Docker-Content-Digest"))
|
|
assert.Equal(t, "application/vnd.oci.image.manifest.v1+json", recorder.Header().Get("Content-Type"))
|
|
assert.Equal(t, "/v2/test/repo/blobs/uploads/uuid123", recorder.Header().Get("Location"))
|
|
assert.Equal(t, "0-511", recorder.Header().Get("Range"))
|
|
assert.Equal(t, "uuid123", recorder.Header().Get("Docker-Upload-Uuid"))
|
|
assert.Equal(t, "registry/2.0", recorder.Header().Get("Docker-Distribution-Api-Version"))
|
|
assert.Equal(t, `"sha256:test123"`, recorder.Header().Get("ETag"))
|
|
assert.Equal(t, http.StatusAccepted, recorder.Code)
|
|
})
|
|
}
|
|
|
|
// TestResponseHeadersForEmptyBlobs tests the core fix for ORAS empty blob support
|
|
func TestResponseHeadersForEmptyBlobs(t *testing.T) {
|
|
t.Run("Content-Length set for empty blob", func(t *testing.T) {
|
|
recorder := httptest.NewRecorder()
|
|
|
|
// This tests the main fix: empty blobs should have Content-Length: 0
|
|
setResponseHeaders(recorder, &containerHeaders{
|
|
Status: http.StatusOK,
|
|
ContentLength: 0, // Empty blob (like empty config in ORAS artifacts)
|
|
ContentDigest: "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
|
|
})
|
|
|
|
// The key fix: Content-Length should be set even for 0-byte blobs
|
|
assert.Equal(t, "0", recorder.Header().Get("Content-Length"))
|
|
assert.Equal(t, "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", recorder.Header().Get("Docker-Content-Digest"))
|
|
assert.Equal(t, "registry/2.0", recorder.Header().Get("Docker-Distribution-Api-Version"))
|
|
assert.Equal(t, `"sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"`, recorder.Header().Get("ETag"))
|
|
assert.Equal(t, http.StatusOK, recorder.Code)
|
|
})
|
|
|
|
t.Run("Content-Length set for regular blob", func(t *testing.T) {
|
|
recorder := httptest.NewRecorder()
|
|
|
|
setResponseHeaders(recorder, &containerHeaders{
|
|
Status: http.StatusOK,
|
|
ContentLength: 1024,
|
|
ContentDigest: "sha256:abcd1234",
|
|
})
|
|
|
|
assert.Equal(t, "1024", recorder.Header().Get("Content-Length"))
|
|
assert.Equal(t, "sha256:abcd1234", recorder.Header().Get("Docker-Content-Digest"))
|
|
})
|
|
|
|
t.Run("All headers set correctly", func(t *testing.T) {
|
|
recorder := httptest.NewRecorder()
|
|
|
|
setResponseHeaders(recorder, &containerHeaders{
|
|
Status: http.StatusAccepted,
|
|
ContentLength: 512,
|
|
ContentDigest: "sha256:test123",
|
|
ContentType: "application/vnd.oci.image.manifest.v1+json",
|
|
Location: "/v2/test/repo/blobs/uploads/uuid123",
|
|
Range: "0-511",
|
|
UploadUUID: "uuid123",
|
|
})
|
|
|
|
assert.Equal(t, "512", recorder.Header().Get("Content-Length"))
|
|
assert.Equal(t, "sha256:test123", recorder.Header().Get("Docker-Content-Digest"))
|
|
assert.Equal(t, "application/vnd.oci.image.manifest.v1+json", recorder.Header().Get("Content-Type"))
|
|
assert.Equal(t, "/v2/test/repo/blobs/uploads/uuid123", recorder.Header().Get("Location"))
|
|
assert.Equal(t, "0-511", recorder.Header().Get("Range"))
|
|
assert.Equal(t, "uuid123", recorder.Header().Get("Docker-Upload-Uuid"))
|
|
assert.Equal(t, "registry/2.0", recorder.Header().Get("Docker-Distribution-Api-Version"))
|
|
assert.Equal(t, `"sha256:test123"`, recorder.Header().Get("ETag"))
|
|
assert.Equal(t, http.StatusAccepted, recorder.Code)
|
|
})
|
|
}
|