fix artifact range requests (#4218)

I noticed that Forgejo does not allow HTTP range requests when downloading artifacts. All other file downloads like releases and packages support them.

So I looked at the code and found that the artifact download endpoint uses a simple io.Copy to serve the file contents instead of using the established `ServeContentByReadSeeker` function which does take range requests into account.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4218
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: ThetaDev <thetadev@magenta.de>
Co-committed-by: ThetaDev <thetadev@magenta.de>

models/fixtures/repo_unit.yml

@@ -788,3 +788,10 @@
   type: 10
   config: "{}"
   created_unix: 946684810
+
+-
+  id: 114
+  repo_id: 4
+  type: 10
+  config: "{}"
+  created_unix: 946684810

routers/api/actions/artifactsv4.go

@@ -97,6 +97,7 @@ import (
 	"code.gitea.io/gitea/modules/storage"
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web"
+	"code.gitea.io/gitea/routers/common"
 	"code.gitea.io/gitea/services/context"
 
 	"google.golang.org/protobuf/encoding/protojson"
@@ -473,9 +474,14 @@ func (r *artifactV4Routes) downloadArtifact(ctx *ArtifactContext) {
 		return
 	}
 
-	file, _ := r.fs.Open(artifact.StoragePath)
+	file, err := r.fs.Open(artifact.StoragePath)
+	if err != nil {
+		log.Error("Error artifact could not be opened: %v", err)
+		ctx.Error(http.StatusInternalServerError, err.Error())
+		return
+	}
 
-	_, _ = io.Copy(ctx.Resp, file)
+	common.ServeContentByReadSeeker(ctx.Base, artifactName, util.ToPointer(artifact.UpdatedUnix.AsTime()), file)
 }
 
 func (r *artifactV4Routes) deleteArtifact(ctx *ArtifactContext) {

routers/web/repo/actions/view.go

@@ -28,6 +28,7 @@ import (
 	"code.gitea.io/gitea/modules/timeutil"
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web"
+	"code.gitea.io/gitea/routers/common"
 	actions_service "code.gitea.io/gitea/services/actions"
 	context_module "code.gitea.io/gitea/services/context"
 
@@ -662,10 +663,8 @@ func ArtifactsDownloadView(ctx *context_module.Context) {
 		}
 	}
 
-	ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(artifactName), artifactName))
-
 	// Artifacts using the v4 backend are stored as a single combined zip file per artifact on the backend
-	// The v4 backend enshures ContentEncoding is set to "application/zip", which is not the case for the old backend
+	// The v4 backend ensures ContentEncoding is set to "application/zip", which is not the case for the old backend
 	if len(artifacts) == 1 && artifacts[0].ArtifactName+".zip" == artifacts[0].ArtifactPath && artifacts[0].ContentEncoding == "application/zip" {
 		art := artifacts[0]
 		if setting.Actions.ArtifactStorage.MinioConfig.ServeDirect {
@@ -680,12 +679,13 @@ func ArtifactsDownloadView(ctx *context_module.Context) {
 			ctx.Error(http.StatusInternalServerError, err.Error())
 			return
 		}
-		_, _ = io.Copy(ctx.Resp, f)
+		common.ServeContentByReadSeeker(ctx.Base, artifactName, util.ToPointer(art.UpdatedUnix.AsTime()), f)
 		return
 	}
 
 	// Artifacts using the v1-v3 backend are stored as multiple individual files per artifact on the backend
 	// Those need to be zipped for download
+	ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(artifactName), artifactName))
 	writer := zip.NewWriter(ctx.Resp)
 	defer writer.Close()
 	for _, art := range artifacts {

tests/integration/api_actions_artifact_v4_test.go

@@ -31,9 +31,7 @@ func toProtoJSON(m protoreflect.ProtoMessage) io.Reader {
 	return &buf
 }
 
-func TestActionsArtifactV4UploadSingleFile(t *testing.T) {
+func uploadArtifact(t *testing.T, body string) string {
-	defer tests.PrepareTestEnv(t)()
-
 	token, err := actions_service.CreateAuthorizationToken(48, 792, 193)
 	assert.NoError(t, err)
 
@@ -55,7 +53,6 @@ func TestActionsArtifactV4UploadSingleFile(t *testing.T) {
 	url := uploadResp.SignedUploadUrl[idx:] + "&comp=block"
 
 	// upload artifact chunk
-	body := strings.Repeat("A", 1024)
 	req = NewRequestWithBody(t, "PUT", url, strings.NewReader(body))
 	MakeRequest(t, req, http.StatusCreated)
 
@@ -76,6 +73,13 @@ func TestActionsArtifactV4UploadSingleFile(t *testing.T) {
 	var finalizeResp actions.FinalizeArtifactResponse
 	protojson.Unmarshal(resp.Body.Bytes(), &finalizeResp)
 	assert.True(t, finalizeResp.Ok)
+	return token
+}
+
+func TestActionsArtifactV4UploadSingleFile(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+	body := strings.Repeat("A", 1024)
+	uploadArtifact(t, body)
 }
 
 func TestActionsArtifactV4UploadSingleFileWrongChecksum(t *testing.T) {
@@ -202,7 +206,52 @@ func TestActionsArtifactV4DownloadSingle(t *testing.T) {
 	req = NewRequest(t, "GET", finalizeResp.SignedUrl)
 	resp = MakeRequest(t, req, http.StatusOK)
 	body := strings.Repeat("A", 1024)
-	assert.Equal(t, resp.Body.String(), body)
+	assert.Equal(t, "bytes", resp.Header().Get("accept-ranges"))
+	assert.Equal(t, body, resp.Body.String())
+
+	// Download artifact via user-facing URL
+	req = NewRequest(t, "GET", "/user5/repo4/actions/runs/188/artifacts/artifact")
+	resp = MakeRequest(t, req, http.StatusOK)
+	assert.Equal(t, "bytes", resp.Header().Get("accept-ranges"))
+	assert.Equal(t, body, resp.Body.String())
+
+	// Partial artifact download
+	req = NewRequest(t, "GET", "/user5/repo4/actions/runs/188/artifacts/artifact").SetHeader("range", "bytes=0-99")
+	resp = MakeRequest(t, req, http.StatusPartialContent)
+	body = strings.Repeat("A", 100)
+	assert.Equal(t, "bytes 0-99/1024", resp.Header().Get("content-range"))
+	assert.Equal(t, body, resp.Body.String())
+}
+
+func TestActionsArtifactV4DownloadRange(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	bstr := strings.Repeat("B", 100)
+	body := strings.Repeat("A", 100) + bstr
+	token := uploadArtifact(t, body)
+
+	// Download (Actions API)
+	req := NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/GetSignedArtifactURL", toProtoJSON(&actions.GetSignedArtifactURLRequest{
+		Name:                    "artifact",
+		WorkflowRunBackendId:    "792",
+		WorkflowJobRunBackendId: "193",
+	})).
+		AddTokenAuth(token)
+	resp := MakeRequest(t, req, http.StatusOK)
+	var finalizeResp actions.GetSignedArtifactURLResponse
+	protojson.Unmarshal(resp.Body.Bytes(), &finalizeResp)
+	assert.NotEmpty(t, finalizeResp.SignedUrl)
+
+	req = NewRequest(t, "GET", finalizeResp.SignedUrl).SetHeader("range", "bytes=100-199")
+	resp = MakeRequest(t, req, http.StatusPartialContent)
+	assert.Equal(t, "bytes 100-199/200", resp.Header().Get("content-range"))
+	assert.Equal(t, bstr, resp.Body.String())
+
+	// Download (user-facing API)
+	req = NewRequest(t, "GET", "/user5/repo4/actions/runs/188/artifacts/artifact").SetHeader("range", "bytes=100-199")
+	resp = MakeRequest(t, req, http.StatusPartialContent)
+	assert.Equal(t, "bytes 100-199/200", resp.Header().Get("content-range"))
+	assert.Equal(t, bstr, resp.Body.String())
 }
 
 func TestActionsArtifactV4Delete(t *testing.T) {