diff --git a/modules/httplib/serve.go b/modules/httplib/serve.go index cd35367bc9..c5f0658d4e 100644 --- a/modules/httplib/serve.go +++ b/modules/httplib/serve.go @@ -35,6 +35,8 @@ type ServeHeaderOptions struct { Filename string CacheDuration time.Duration // defaults to 5 minutes LastModified time.Time + AdditionalHeaders http.Header + RedirectStatusCode int } // ServeSetHeaders sets necessary content serve headers @@ -82,6 +84,12 @@ func ServeSetHeaders(w http.ResponseWriter, opts *ServeHeaderOptions) { // http.TimeFormat required a UTC time, refer to https://pkg.go.dev/net/http#TimeFormat header.Set("Last-Modified", opts.LastModified.UTC().Format(http.TimeFormat)) } + + if opts.AdditionalHeaders != nil { + for k, v := range opts.AdditionalHeaders { + header[k] = v + } + } } // ServeData download file from io.Reader diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go index 5276dd5706..4d59e391a5 100644 --- a/routers/api/packages/container/container.go +++ b/routers/api/packages/container/container.go @@ -691,33 +691,30 @@ func DeleteManifest(ctx *context.Context) { func serveBlob(ctx *context.Context, pfd *packages_model.PackageFileDescriptor) { serveDirectReqParams := make(url.Values) serveDirectReqParams.Set("response-content-type", pfd.Properties.GetByName(container_module.PropertyMediaType)) - s, u, _, err := packages_service.GetPackageBlobStream(ctx, pfd.File, pfd.Blob, serveDirectReqParams) + s, u, pf, err := packages_service.GetPackageBlobStream(ctx, pfd.File, pfd.Blob, serveDirectReqParams) if err != nil { + if errors.Is(err, packages_model.ErrPackageFileNotExist) { + apiError(ctx, http.StatusNotFound, err) + return + } apiError(ctx, http.StatusInternalServerError, err) return } - headers := &containerHeaders{ - ContentDigest: pfd.Properties.GetByName(container_module.PropertyDigest), - ContentType: pfd.Properties.GetByName(container_module.PropertyMediaType), - ContentLength: pfd.Blob.Size, - Status: http.StatusOK, + opts := &context.ServeHeaderOptions{ + ContentType: pfd.Properties.GetByName(container_module.PropertyMediaType), + RedirectStatusCode: http.StatusTemporaryRedirect, + AdditionalHeaders: map[string][]string{ + "Docker-Distribution-Api-Version": {"registry/2.0"}, + }, } - if u != nil { - headers.Status = http.StatusTemporaryRedirect - headers.Location = u.String() - - setResponseHeaders(ctx.Resp, headers) - return + if d := pfd.Properties.GetByName(container_module.PropertyDigest); d != "" { + opts.AdditionalHeaders["Docker-Content-Digest"] = []string{d} + opts.AdditionalHeaders["ETag"] = []string{fmt.Sprintf(`"%s"`, d)} } - defer s.Close() - - setResponseHeaders(ctx.Resp, headers) - if _, err := io.Copy(ctx.Resp, s); err != nil { - log.Error("Error whilst copying content to response: %v", err) - } + helper.ServePackageFile(ctx, s, u, pf, opts) } // https://github.com/opencontainers/distribution-spec/blob/main/spec.md#content-discovery @@ -725,7 +722,7 @@ func GetTagList(ctx *context.Context) { image := ctx.Params("image") if _, err := packages_model.GetPackageByName(ctx, ctx.Package.Owner.ID, packages_model.TypeContainer, image); err != nil { - if err == packages_model.ErrPackageNotExist { + if errors.Is(err, packages_model.ErrPackageNotExist) { apiErrorDefined(ctx, errNameUnknown) } else { apiError(ctx, http.StatusInternalServerError, err) diff --git a/routers/api/packages/helper/helper.go b/routers/api/packages/helper/helper.go index 99c0867bbb..f9b91d9a09 100644 --- a/routers/api/packages/helper/helper.go +++ b/routers/api/packages/helper/helper.go @@ -39,16 +39,9 @@ func LogAndProcessError(ctx *context.Context, status int, obj any, cb func(strin } } -// Serves the content of the package file +// ServePackageFile Serves the content of the package file // If the url is set it will redirect the request, otherwise the content is copied to the response. func ServePackageFile(ctx *context.Context, s io.ReadSeekCloser, u *url.URL, pf *packages_model.PackageFile, forceOpts ...*context.ServeHeaderOptions) { - if u != nil { - ctx.Redirect(u.String()) - return - } - - defer s.Close() - var opts *context.ServeHeaderOptions if len(forceOpts) > 0 { opts = forceOpts[0] @@ -59,5 +52,12 @@ func ServePackageFile(ctx *context.Context, s io.ReadSeekCloser, u *url.URL, pf } } + if u != nil { + ctx.Redirect(u.String(), opts.RedirectStatusCode) + return + } + + defer s.Close() + ctx.ServeContent(s, opts) } diff --git a/services/context/base.go b/services/context/base.go index 0275ea8a99..dc3d226bb0 100644 --- a/services/context/base.go +++ b/services/context/base.go @@ -250,7 +250,7 @@ func (b *Base) PlainText(status int, text string) { // Redirect redirects the request func (b *Base) Redirect(location string, status ...int) { code := http.StatusSeeOther - if len(status) == 1 { + if len(status) == 1 && status[0] > 0 { code = status[0] } diff --git a/services/context/base_test.go b/services/context/base_test.go index bf746766d9..9e058d8f24 100644 --- a/services/context/base_test.go +++ b/services/context/base_test.go @@ -36,6 +36,7 @@ func TestRedirect(t *testing.T) { cleanup() has := resp.Header().Get("Set-Cookie") == "i_like_gitea=dummy" assert.Equal(t, c.keep, has, "url = %q", c.url) + assert.Equal(t, http.StatusSeeOther, resp.Code) } req, _ = http.NewRequest("GET", "/", nil) @@ -47,3 +48,24 @@ func TestRedirect(t *testing.T) { assert.Equal(t, "/other", resp.Header().Get("HX-Redirect")) assert.Equal(t, http.StatusNoContent, resp.Code) } + +func TestRedirectOptionalStatus(t *testing.T) { + defer test.MockVariableValue(&setting.AppURL, "http://localhost:3000/")() + req, _ := http.NewRequest("GET", "/", nil) + + cases := []struct { + expected int + actual int + }{ + {expected: 303}, + {http.StatusTemporaryRedirect, 307}, + {http.StatusPermanentRedirect, 308}, + } + for _, c := range cases { + resp := httptest.NewRecorder() + b, cleanup := NewBaseContext(resp, req) + b.Redirect("/", c.actual) + cleanup() + assert.Equal(t, c.expected, resp.Code) + } +} diff --git a/tests/integration/api_packages_container_test.go b/tests/integration/api_packages_container_test.go index 0e977e9ae7..e3f7d010b3 100644 --- a/tests/integration/api_packages_container_test.go +++ b/tests/integration/api_packages_container_test.go @@ -430,14 +430,19 @@ func TestPackageContainer(t *testing.T) { assert.Equal(t, manifestDigest, resp.Header().Get("Docker-Content-Digest")) }) - t.Run("GetManifest", func(t *testing.T) { + t.Run("GetManifest unknown-tag", func(t *testing.T) { defer tests.PrintCurrentTest(t)() req := NewRequest(t, "GET", fmt.Sprintf("%s/manifests/unknown-tag", url)). AddTokenAuth(userToken) MakeRequest(t, req, http.StatusNotFound) + }) - req = NewRequest(t, "GET", fmt.Sprintf("%s/manifests/%s", url, tag)). + t.Run("GetManifest serv indirect", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + defer test.MockVariableValue(&setting.Packages.Storage.MinioConfig.ServeDirect, false)() + + req := NewRequest(t, "GET", fmt.Sprintf("%s/manifests/%s", url, tag)). AddTokenAuth(userToken) resp := MakeRequest(t, req, http.StatusOK) @@ -446,6 +451,25 @@ func TestPackageContainer(t *testing.T) { assert.Equal(t, manifestDigest, resp.Header().Get("Docker-Content-Digest")) assert.Equal(t, manifestContent, resp.Body.String()) }) + + t.Run("GetManifest serv direct", func(t *testing.T) { + if setting.Packages.Storage.Type != setting.MinioStorageType { + t.Skip("Test skipped for non-Minio-storage.") + return + } + + defer tests.PrintCurrentTest(t)() + defer test.MockVariableValue(&setting.Packages.Storage.MinioConfig.ServeDirect, true)() + + req := NewRequest(t, "GET", fmt.Sprintf("%s/manifests/%s", url, tag)). + AddTokenAuth(userToken) + resp := MakeRequest(t, req, http.StatusTemporaryRedirect) + + assert.Empty(t, resp.Header().Get("Content-Length")) + assert.NotEmpty(t, resp.Header().Get("Location")) + assert.Equal(t, "text/html; charset=utf-8", resp.Header().Get("Content-Type")) + assert.Empty(t, resp.Header().Get("Docker-Content-Digest")) + }) }) }