From ea4e445316a9ee39fb2a11b68b550bbd2b63ac65 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Mon, 2 Mar 2026 14:40:56 +0100 Subject: [PATCH 01/26] Fix artifacts v4 backend upload problems * Use base64.RawURLEncoding to avoid equal sign * using the nodejs package they seem to get lost * Support uploads with unspecified length * Support uploads with a single named blockid * without requiring a blockmap --- routers/api/actions/artifacts_chunks.go | 80 +++++++++--- routers/api/actions/artifactsv4.go | 57 ++++---- .../api_actions_artifact_v4_test.go | 123 ++++++++++++------ 3 files changed, 166 insertions(+), 94 deletions(-) diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index 708931d1ac1c8..bdea92380153f 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -21,6 +21,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/storage" + "code.gitea.io/gitea/modules/util" ) func saveUploadChunkBase(st storage.ObjectStorage, ctx *ArtifactContext, @@ -54,7 +55,7 @@ func saveUploadChunkBase(st storage.ObjectStorage, ctx *ArtifactContext, checkErr = errors.New("md5 not match") } } - if writtenSize != contentSize { + if writtenSize != contentSize && contentSize != -1 { checkErr = errors.Join(checkErr, fmt.Errorf("writtenSize %d not match contentSize %d", writtenSize, contentSize)) } if checkErr != nil { @@ -88,7 +89,7 @@ func appendUploadChunk(st storage.ObjectStorage, ctx *ArtifactContext, start, contentSize, runID int64, ) (int64, error) { end := start + contentSize - 1 - return saveUploadChunkBase(st, ctx, artifact, contentSize, runID, start, end, contentSize, false) + return saveUploadChunkBase(st, ctx, artifact, util.Iif(contentSize == 0, -1, contentSize), runID, start, end, contentSize, false) } type chunkFileItem struct { @@ -110,6 +111,13 @@ func listChunksByRunID(st storage.ObjectStorage, runID int64) (map[int64][]*chun if _, err := fmt.Sscanf(baseName, "%d-%d-%d-%d.chunk", &item.RunID, &item.ArtifactID, &item.Start, &item.End); err != nil { return fmt.Errorf("parse content range error: %v", err) } + if (item.End + 1 - item.Start) == 0 { + fi, err := st.Stat(fpath) + if err != nil { + return err + } + item.End = item.Start + fi.Size() - 1 + } chunks = append(chunks, &item) return nil }); err != nil { @@ -126,10 +134,13 @@ func listChunksByRunID(st storage.ObjectStorage, runID int64) (map[int64][]*chun func listChunksByRunIDV4(st storage.ObjectStorage, runID, artifactID int64, blist *BlockList) ([]*chunkFileItem, error) { storageDir := fmt.Sprintf("tmpv4%d", runID) var chunks []*chunkFileItem - chunkMap := map[string]*chunkFileItem{} - dummy := &chunkFileItem{} - for _, name := range blist.Latest { - chunkMap[name] = dummy + var chunkMap map[string]*chunkFileItem + if blist != nil { + chunkMap = map[string]*chunkFileItem{} + dummy := &chunkFileItem{} + for _, name := range blist.Latest { + chunkMap[name] = dummy + } } if err := st.IterateObjects(storageDir, func(fpath string, obj storage.Object) error { baseName := filepath.Base(fpath) @@ -141,33 +152,64 @@ func listChunksByRunIDV4(st storage.ObjectStorage, runID, artifactID int64, blis item := chunkFileItem{Path: storageDir + "/" + baseName, ArtifactID: artifactID} var size int64 var b64chunkName string - if _, err := fmt.Sscanf(baseName, "block-%d-%d-%s", &item.RunID, &size, &b64chunkName); err != nil { - return fmt.Errorf("parse content range error: %v", err) + var fArtifactID int64 + if _, err := fmt.Sscanf(baseName, "block-%d-%d-%d-%s", &item.RunID, &fArtifactID, &size, &b64chunkName); err != nil { + log.Warn("parse content range error: %v", err) + return nil } rchunkName, err := base64.URLEncoding.DecodeString(b64chunkName) if err != nil { - return fmt.Errorf("failed to parse chunkName: %v", err) + log.Warn("failed to parse chunkName: %v", err) + return nil + } + if fArtifactID != artifactID { + return nil + } + if size == 0 { + fi, err := st.Stat(fpath) + if err != nil { + return err + } + size = fi.Size() } chunkName := string(rchunkName) item.End = item.Start + size - 1 + // Single chunk upload with blockid if _, ok := chunkMap[chunkName]; ok { chunkMap[chunkName] = &item + } else if chunkMap == nil { + if chunks != nil { + return errors.New("blockmap is required for chunks > 1") + } + chunks = []*chunkFileItem{&item} } return nil - }); err != nil { + }); err != nil && (chunks != nil || blist != nil) { return nil, err - } - for i, name := range blist.Latest { - chunk, ok := chunkMap[name] - if !ok || chunk.Path == "" { - return nil, fmt.Errorf("missing Chunk (%d/%d): %s", i, len(blist.Latest), name) + } else if blist == nil && chunks == nil { + var chunkMap map[int64][]*chunkFileItem + chunkMap, err = listChunksByRunID(st, runID) + if err != nil { + return nil, err } - chunks = append(chunks, chunk) - if i > 0 { - chunk.Start = chunkMap[blist.Latest[i-1]].End + 1 - chunk.End += chunk.Start + chunks = chunkMap[artifactID] + } + if blist != nil { + for i, name := range blist.Latest { + chunk, ok := chunkMap[name] + if !ok || chunk.Path == "" { + return nil, fmt.Errorf("missing Chunk (%d/%d): %s", i, len(blist.Latest), name) + } + chunks = append(chunks, chunk) + if i > 0 { + chunk.Start = chunkMap[blist.Latest[i-1]].End + 1 + chunk.End += chunk.Start + } } } + if len(chunks) < 1 { + return nil, errors.New("no Chunk found") + } return chunks, nil } diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 337e6bf26c25b..698a37071ea14 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -90,6 +90,7 @@ import ( "crypto/sha256" "encoding/base64" "encoding/xml" + "errors" "fmt" "io" "net/http" @@ -170,7 +171,7 @@ func (r artifactV4Routes) buildSignature(endp, expires, artifactName string, tas func (r artifactV4Routes) buildArtifactURL(ctx *ArtifactContext, endp, artifactName string, taskID, artifactID int64) string { expires := time.Now().Add(60 * time.Minute).Format("2006-01-02 15:04:05.999999999 -0700 MST") uploadURL := strings.TrimSuffix(httplib.GuessCurrentAppURL(ctx), "/") + strings.TrimSuffix(r.prefix, "/") + - "/" + endp + "?sig=" + base64.URLEncoding.EncodeToString(r.buildSignature(endp, expires, artifactName, taskID, artifactID)) + "&expires=" + url.QueryEscape(expires) + "&artifactName=" + url.QueryEscape(artifactName) + "&taskID=" + strconv.FormatInt(taskID, 10) + "&artifactID=" + strconv.FormatInt(artifactID, 10) + "/" + endp + "?sig=" + base64.RawURLEncoding.EncodeToString(r.buildSignature(endp, expires, artifactName, taskID, artifactID)) + "&expires=" + url.QueryEscape(expires) + "&artifactName=" + url.QueryEscape(artifactName) + "&taskID=" + strconv.FormatInt(taskID, 10) + "&artifactID=" + strconv.FormatInt(artifactID, 10) return uploadURL } @@ -180,7 +181,7 @@ func (r artifactV4Routes) verifySignature(ctx *ArtifactContext, endp string) (*a sig := ctx.Req.URL.Query().Get("sig") expires := ctx.Req.URL.Query().Get("expires") artifactName := ctx.Req.URL.Query().Get("artifactName") - dsig, _ := base64.URLEncoding.DecodeString(sig) + dsig, _ := base64.RawURLEncoding.DecodeString(sig) taskID, _ := strconv.ParseInt(rawTaskID, 10, 64) artifactID, _ := strconv.ParseInt(rawArtifactID, 10, 64) @@ -303,16 +304,15 @@ func (r *artifactV4Routes) uploadArtifact(ctx *ArtifactContext) { comp := ctx.Req.URL.Query().Get("comp") switch comp { case "block", "appendBlock": + // get artifact by name + artifact, err := r.getArtifactByName(ctx, task.Job.RunID, artifactName) + if err != nil { + log.Error("Error artifact not found: %v", err) + ctx.HTTPError(http.StatusNotFound, "Error artifact not found") + return + } blockid := ctx.Req.URL.Query().Get("blockid") if blockid == "" { - // get artifact by name - artifact, err := r.getArtifactByName(ctx, task.Job.RunID, artifactName) - if err != nil { - log.Error("Error artifact not found: %v", err) - ctx.HTTPError(http.StatusNotFound, "Error artifact not found") - return - } - _, err = appendUploadChunk(r.fs, ctx, artifact, artifact.FileSize, ctx.Req.ContentLength, artifact.RunID) if err != nil { log.Error("Error runner api getting task: task is not running") @@ -327,7 +327,7 @@ func (r *artifactV4Routes) uploadArtifact(ctx *ArtifactContext) { return } } else { - _, err := r.fs.Save(fmt.Sprintf("tmpv4%d/block-%d-%d-%s", task.Job.RunID, task.Job.RunID, ctx.Req.ContentLength, base64.URLEncoding.EncodeToString([]byte(blockid))), ctx.Req.Body, -1) + _, err := r.fs.Save(fmt.Sprintf("tmpv4%d/block-%d-%d-%d-%s", task.Job.RunID, task.Job.RunID, artifact.ID, ctx.Req.ContentLength, base64.URLEncoding.EncodeToString([]byte(blockid))), ctx.Req.Body, -1) if err != nil { log.Error("Error runner api getting task: task is not running") ctx.HTTPError(http.StatusInternalServerError, "Error runner api getting task: task is not running") @@ -371,7 +371,10 @@ func (r *artifactV4Routes) readBlockList(runID, artifactID int64) (*BlockList, e if delerr != nil { log.Warn("Failed to delete blockList %s: %v", blockListName, delerr) } - return blockList, err + if err != nil { + return nil, err + } + return blockList, nil } func (r *artifactV4Routes) finalizeArtifact(ctx *ArtifactContext) { @@ -394,31 +397,15 @@ func (r *artifactV4Routes) finalizeArtifact(ctx *ArtifactContext) { } var chunks []*chunkFileItem - blockList, err := r.readBlockList(runID, artifact.ID) + blockList, blockListErr := r.readBlockList(runID, artifact.ID) + chunks, err = listChunksByRunIDV4(r.fs, runID, artifact.ID, blockList) if err != nil { - log.Warn("Failed to read BlockList, fallback to old behavior: %v", err) - chunkMap, err := listChunksByRunID(r.fs, runID) - if err != nil { - log.Error("Error merge chunks: %v", err) - ctx.HTTPError(http.StatusInternalServerError, "Error merge chunks") - return - } - chunks, ok = chunkMap[artifact.ID] - if !ok { - log.Error("Error merge chunks") - ctx.HTTPError(http.StatusInternalServerError, "Error merge chunks") - return - } - } else { - chunks, err = listChunksByRunIDV4(r.fs, runID, artifact.ID, blockList) - if err != nil { - log.Error("Error merge chunks: %v", err) - ctx.HTTPError(http.StatusInternalServerError, "Error merge chunks") - return - } - artifact.FileSize = chunks[len(chunks)-1].End + 1 - artifact.FileCompressedSize = chunks[len(chunks)-1].End + 1 + log.Error("Error merge chunks: %v", errors.Join(blockListErr, err)) + ctx.HTTPError(http.StatusInternalServerError, "Error merge chunks") + return } + artifact.FileSize = chunks[len(chunks)-1].End + 1 + artifact.FileCompressedSize = chunks[len(chunks)-1].End + 1 checksum := "" if req.Hash != nil { diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index 3db8bbb82e78c..0349fd712a4e3 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -6,6 +6,7 @@ package integration import ( "bytes" "crypto/sha256" + "encoding/base64" "encoding/hex" "encoding/xml" "fmt" @@ -45,45 +46,87 @@ func TestActionsArtifactV4UploadSingleFile(t *testing.T) { token, err := actions_service.CreateAuthorizationToken(48, 792, 193) assert.NoError(t, err) - // acquire artifact upload url - req := NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/CreateArtifact", toProtoJSON(&actions.CreateArtifactRequest{ - Version: 4, - Name: "artifact", - WorkflowRunBackendId: "792", - WorkflowJobRunBackendId: "193", - })).AddTokenAuth(token) - resp := MakeRequest(t, req, http.StatusOK) - var uploadResp actions.CreateArtifactResponse - protojson.Unmarshal(resp.Body.Bytes(), &uploadResp) - assert.True(t, uploadResp.Ok) - assert.Contains(t, uploadResp.SignedUploadUrl, "/twirp/github.actions.results.api.v1.ArtifactService/UploadArtifact") - - // get upload url - idx := strings.Index(uploadResp.SignedUploadUrl, "/twirp/") - 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) - - t.Logf("Create artifact confirm") - - sha := sha256.Sum256([]byte(body)) + table := []struct { + name string + version int32 + blockID bool + noLength bool + }{ + { + name: "artifact", + version: 4, + }, + { + name: "artifact2", + version: 4, + blockID: true, + }, + { + name: "artifact3", + version: 4, + noLength: true, + }, + { + name: "artifact3", + version: 4, + blockID: true, + noLength: true, + }, + { + name: "artifact4", + version: 7, + blockID: true, + }, + } - // confirm artifact upload - req = NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/FinalizeArtifact", toProtoJSON(&actions.FinalizeArtifactRequest{ - Name: "artifact", - Size: 1024, - Hash: wrapperspb.String("sha256:" + hex.EncodeToString(sha[:])), - WorkflowRunBackendId: "792", - WorkflowJobRunBackendId: "193", - })). - AddTokenAuth(token) - resp = MakeRequest(t, req, http.StatusOK) - var finalizeResp actions.FinalizeArtifactResponse - protojson.Unmarshal(resp.Body.Bytes(), &finalizeResp) - assert.True(t, finalizeResp.Ok) + for _, entry := range table { + // acquire artifact upload url + req := NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/CreateArtifact", toProtoJSON(&actions.CreateArtifactRequest{ + Version: entry.version, + Name: entry.name, + WorkflowRunBackendId: "792", + WorkflowJobRunBackendId: "193", + })).AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + var uploadResp actions.CreateArtifactResponse + protojson.Unmarshal(resp.Body.Bytes(), &uploadResp) + assert.True(t, uploadResp.Ok) + assert.Contains(t, uploadResp.SignedUploadUrl, "/twirp/github.actions.results.api.v1.ArtifactService/UploadArtifact") + + // get upload url + idx := strings.Index(uploadResp.SignedUploadUrl, "/twirp/") + url := uploadResp.SignedUploadUrl[idx:] + "&comp=block" + if entry.blockID { + url += "&blockid=" + base64.RawURLEncoding.EncodeToString([]byte("SOME_BIG_BLOCK_ID")) + } + + // upload artifact chunk + body := strings.Repeat("A", 1024) + var bodyReader io.Reader = strings.NewReader(body) + if entry.noLength { + bodyReader = io.MultiReader(bodyReader) + } + req = NewRequestWithBody(t, "PUT", url, bodyReader) + MakeRequest(t, req, http.StatusCreated) + + t.Logf("Create artifact confirm") + + sha := sha256.Sum256([]byte(body)) + + // confirm artifact upload + req = NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/FinalizeArtifact", toProtoJSON(&actions.FinalizeArtifactRequest{ + Name: entry.name, + Size: 1024, + Hash: wrapperspb.String("sha256:" + hex.EncodeToString(sha[:])), + WorkflowRunBackendId: "792", + WorkflowJobRunBackendId: "193", + })). + AddTokenAuth(token) + resp = MakeRequest(t, req, http.StatusOK) + var finalizeResp actions.FinalizeArtifactResponse + protojson.Unmarshal(resp.Body.Bytes(), &finalizeResp) + assert.True(t, finalizeResp.Ok) + } } func TestActionsArtifactV4UploadSingleFileWrongChecksum(t *testing.T) { @@ -312,7 +355,7 @@ func TestActionsArtifactV4DownloadSingle(t *testing.T) { token, err := actions_service.CreateAuthorizationToken(48, 792, 193) assert.NoError(t, err) - // acquire artifact upload url + // list artifacts by name req := NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/ListArtifacts", toProtoJSON(&actions.ListArtifactsRequest{ NameFilter: wrapperspb.String("artifact-v4-download"), WorkflowRunBackendId: "792", @@ -323,7 +366,7 @@ func TestActionsArtifactV4DownloadSingle(t *testing.T) { protojson.Unmarshal(resp.Body.Bytes(), &listResp) assert.Len(t, listResp.Artifacts, 1) - // confirm artifact upload + // acquire artifact download url req = NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/GetSignedArtifactURL", toProtoJSON(&actions.GetSignedArtifactURLRequest{ Name: "artifact-v4-download", WorkflowRunBackendId: "792", From 871f155d6bcadf489475b346bac9c034961f6b9d Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Mon, 2 Mar 2026 17:24:48 +0100 Subject: [PATCH 02/26] fixes --- routers/api/actions/artifacts_chunks.go | 4 ++-- routers/api/actions/artifactsv4.go | 2 ++ tests/integration/api_actions_artifact_v4_test.go | 4 ++-- tests/integration/integration_test.go | 4 ++++ 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index bdea92380153f..fcc3f3186f367 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -111,7 +111,7 @@ func listChunksByRunID(st storage.ObjectStorage, runID int64) (map[int64][]*chun if _, err := fmt.Sscanf(baseName, "%d-%d-%d-%d.chunk", &item.RunID, &item.ArtifactID, &item.Start, &item.End); err != nil { return fmt.Errorf("parse content range error: %v", err) } - if (item.End + 1 - item.Start) == 0 { + if (item.End + 1 - item.Start) <= 0 { fi, err := st.Stat(fpath) if err != nil { return err @@ -165,7 +165,7 @@ func listChunksByRunIDV4(st storage.ObjectStorage, runID, artifactID int64, blis if fArtifactID != artifactID { return nil } - if size == 0 { + if size <= 0 { fi, err := st.Stat(fpath) if err != nil { return err diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 698a37071ea14..4a90d8f822de6 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -367,6 +367,8 @@ func (r *artifactV4Routes) readBlockList(runID, artifactID int64) (*BlockList, e blockList := &BlockList{} err = xdec.Decode(blockList) + _ = s.Close() + delerr := r.fs.Delete(blockListName) if delerr != nil { log.Warn("Failed to delete blockList %s: %v", blockListName, delerr) diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index 0349fd712a4e3..874c2caf8a8d7 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -67,13 +67,13 @@ func TestActionsArtifactV4UploadSingleFile(t *testing.T) { noLength: true, }, { - name: "artifact3", + name: "artifact4", version: 4, blockID: true, noLength: true, }, { - name: "artifact4", + name: "artifact5", version: 7, blockID: true, }, diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index ca1e094ac27df..8c2942a01232c 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -341,6 +341,10 @@ func MakeRequest(t testing.TB, rw *RequestWrapper, expectedStatus int) *httptest if req.RemoteAddr == "" { req.RemoteAddr = "test-mock:12345" } + // Ensure unknown contentLength is seen as -1 + if req.Body != nil && req.ContentLength == 0 { + req.ContentLength = -1 + } testWebRoutes.ServeHTTP(recorder, req) if expectedStatus != NoExpectedStatus { if expectedStatus != recorder.Code { From 15561a165befa8545007749aa52707c7afa18116 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Mon, 2 Mar 2026 20:06:10 +0100 Subject: [PATCH 03/26] Test appending multiple times without content-length --- routers/api/actions/artifacts_chunks.go | 9 ++- routers/api/actions/artifactsv4.go | 24 +++--- .../api_actions_artifact_v4_test.go | 73 +++++++++++++++---- 3 files changed, 80 insertions(+), 26 deletions(-) diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index fcc3f3186f367..c8dc1c3f61263 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -65,9 +65,9 @@ func saveUploadChunkBase(st storage.ObjectStorage, ctx *ArtifactContext, return -1, checkErr } log.Info("[artifact] save chunk %s, size: %d, artifact id: %d, start: %d, end: %d", - storagePath, contentSize, artifact.ID, start, end) - // return chunk total size - return length, nil + storagePath, writtenSize, artifact.ID, start, end) + // return chunk total size if length is provided, otherwise return writtenSize + return util.Iif(length != -1, length, writtenSize), nil } func saveUploadChunk(st storage.ObjectStorage, ctx *ArtifactContext, @@ -84,12 +84,13 @@ func saveUploadChunk(st storage.ObjectStorage, ctx *ArtifactContext, return saveUploadChunkBase(st, ctx, artifact, contentSize, runID, start, end, length, true) } +// Returns uploaded length func appendUploadChunk(st storage.ObjectStorage, ctx *ArtifactContext, artifact *actions.ActionArtifact, start, contentSize, runID int64, ) (int64, error) { end := start + contentSize - 1 - return saveUploadChunkBase(st, ctx, artifact, util.Iif(contentSize == 0, -1, contentSize), runID, start, end, contentSize, false) + return saveUploadChunkBase(st, ctx, artifact, contentSize, runID, start, end, -1, false) } type chunkFileItem struct { diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 4a90d8f822de6..218c6f5c5601a 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -313,14 +313,14 @@ func (r *artifactV4Routes) uploadArtifact(ctx *ArtifactContext) { } blockid := ctx.Req.URL.Query().Get("blockid") if blockid == "" { - _, err = appendUploadChunk(r.fs, ctx, artifact, artifact.FileSize, ctx.Req.ContentLength, artifact.RunID) + uploadedLength, err := appendUploadChunk(r.fs, ctx, artifact, artifact.FileSize, ctx.Req.ContentLength, artifact.RunID) if err != nil { - log.Error("Error runner api getting task: task is not running") - ctx.HTTPError(http.StatusInternalServerError, "Error runner api getting task: task is not running") + log.Error("Error appending Chunk %v", err) + ctx.HTTPError(http.StatusInternalServerError, "Error appending Chunk") return } - artifact.FileCompressedSize += ctx.Req.ContentLength - artifact.FileSize += ctx.Req.ContentLength + artifact.FileCompressedSize += uploadedLength + artifact.FileSize += uploadedLength if err := actions.UpdateArtifactByID(ctx, artifact.ID, artifact); err != nil { log.Error("Error UpdateArtifactByID: %v", err) ctx.HTTPError(http.StatusInternalServerError, "Error UpdateArtifactByID") @@ -329,8 +329,8 @@ func (r *artifactV4Routes) uploadArtifact(ctx *ArtifactContext) { } else { _, err := r.fs.Save(fmt.Sprintf("tmpv4%d/block-%d-%d-%d-%s", task.Job.RunID, task.Job.RunID, artifact.ID, ctx.Req.ContentLength, base64.URLEncoding.EncodeToString([]byte(blockid))), ctx.Req.Body, -1) if err != nil { - log.Error("Error runner api getting task: task is not running") - ctx.HTTPError(http.StatusInternalServerError, "Error runner api getting task: task is not running") + log.Error("Error uploading block blob %v", err) + ctx.HTTPError(http.StatusInternalServerError, "Error uploading block blob") return } } @@ -340,8 +340,8 @@ func (r *artifactV4Routes) uploadArtifact(ctx *ArtifactContext) { artifactID, _ := strconv.ParseInt(rawArtifactID, 10, 64) _, err := r.fs.Save(fmt.Sprintf("tmpv4%d/%d-%d-blocklist", task.Job.RunID, task.Job.RunID, artifactID), ctx.Req.Body, -1) if err != nil { - log.Error("Error runner api getting task: task is not running") - ctx.HTTPError(http.StatusInternalServerError, "Error runner api getting task: task is not running") + log.Error("Error uploading blocklist %v", err) + ctx.HTTPError(http.StatusInternalServerError, "Error uploading blocklist") return } ctx.JSON(http.StatusCreated, "created") @@ -409,6 +409,12 @@ func (r *artifactV4Routes) finalizeArtifact(ctx *ArtifactContext) { artifact.FileSize = chunks[len(chunks)-1].End + 1 artifact.FileCompressedSize = chunks[len(chunks)-1].End + 1 + if req.Size != artifact.FileSize { + log.Error("Error merge chunks size mismatch") + ctx.HTTPError(http.StatusInternalServerError, "Error merge chunks size mismatch") + return + } + checksum := "" if req.Hash != nil { checksum = req.Hash.Value diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index 874c2caf8a8d7..5a55959d4697d 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -23,6 +23,7 @@ import ( "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/storage" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/routers/api/actions" actions_service "code.gitea.io/gitea/services/actions" @@ -51,6 +52,7 @@ func TestActionsArtifactV4UploadSingleFile(t *testing.T) { version int32 blockID bool noLength bool + append int }{ { name: "artifact", @@ -77,6 +79,25 @@ func TestActionsArtifactV4UploadSingleFile(t *testing.T) { version: 7, blockID: true, }, + { + name: "artifact6", + version: 7, + append: 2, + noLength: true, + }, + { + name: "artifact7", + version: 7, + append: 3, + blockID: true, + noLength: true, + }, + { + name: "artifact8", + version: 7, + append: 4, + blockID: true, + }, } for _, entry := range table { @@ -93,30 +114,56 @@ func TestActionsArtifactV4UploadSingleFile(t *testing.T) { assert.True(t, uploadResp.Ok) assert.Contains(t, uploadResp.SignedUploadUrl, "/twirp/github.actions.results.api.v1.ArtifactService/UploadArtifact") + h := sha256.New() + + blocks := make([]string, 0, util.Iif(entry.blockID, entry.append+1, 0)) + // get upload url idx := strings.Index(uploadResp.SignedUploadUrl, "/twirp/") - url := uploadResp.SignedUploadUrl[idx:] + "&comp=block" - if entry.blockID { - url += "&blockid=" + base64.RawURLEncoding.EncodeToString([]byte("SOME_BIG_BLOCK_ID")) + for i := range entry.append + 1 { + url := uploadResp.SignedUploadUrl[idx:] + // See https://learn.microsoft.com/en-us/rest/api/storageservices/append-block + // See https://learn.microsoft.com/en-us/rest/api/storageservices/put-block + if entry.blockID { + blockID := base64.RawURLEncoding.EncodeToString([]byte(fmt.Sprint("SOME_BIG_BLOCK_ID_", i))) + blocks = append(blocks, blockID) + url += "&comp=block&blockid=" + blockID + } else { + url += "&comp=appendBlock" + } + + // upload artifact chunk + body := strings.Repeat("A", 1024) + _, _ = h.Write([]byte(body)) + var bodyReader io.Reader = strings.NewReader(body) + if entry.noLength { + bodyReader = io.MultiReader(bodyReader) + } + req = NewRequestWithBody(t, "PUT", url, bodyReader) + MakeRequest(t, req, http.StatusCreated) } - // upload artifact chunk - body := strings.Repeat("A", 1024) - var bodyReader io.Reader = strings.NewReader(body) - if entry.noLength { - bodyReader = io.MultiReader(bodyReader) + if entry.blockID && entry.append > 0 { + // https://learn.microsoft.com/en-us/rest/api/storageservices/put-block-list + blockListURL := uploadResp.SignedUploadUrl[idx:] + "&comp=blocklist" + // upload artifact blockList + blockList := &actions.BlockList{ + Latest: blocks, + } + rawBlockList, err := xml.Marshal(blockList) + assert.NoError(t, err) + req = NewRequestWithBody(t, "PUT", blockListURL, bytes.NewReader(rawBlockList)) + MakeRequest(t, req, http.StatusCreated) } - req = NewRequestWithBody(t, "PUT", url, bodyReader) - MakeRequest(t, req, http.StatusCreated) - t.Logf("Create artifact confirm") + sha := h.Sum(nil) - sha := sha256.Sum256([]byte(body)) + t.Logf("Create artifact confirm") // confirm artifact upload req = NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/FinalizeArtifact", toProtoJSON(&actions.FinalizeArtifactRequest{ Name: entry.name, - Size: 1024, + Size: int64(entry.append+1) * 1024, Hash: wrapperspb.String("sha256:" + hex.EncodeToString(sha[:])), WorkflowRunBackendId: "792", WorkflowJobRunBackendId: "193", From f2d184567b0a26911ebad6db5db402071e68a991 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Mon, 2 Mar 2026 20:38:00 +0100 Subject: [PATCH 04/26] lint --- tests/integration/api_actions_artifact_v4_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index 5a55959d4697d..75abcadfd28f9 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -125,7 +125,7 @@ func TestActionsArtifactV4UploadSingleFile(t *testing.T) { // See https://learn.microsoft.com/en-us/rest/api/storageservices/append-block // See https://learn.microsoft.com/en-us/rest/api/storageservices/put-block if entry.blockID { - blockID := base64.RawURLEncoding.EncodeToString([]byte(fmt.Sprint("SOME_BIG_BLOCK_ID_", i))) + blockID := base64.RawURLEncoding.EncodeToString(fmt.Append([]byte("SOME_BIG_BLOCK_ID_"), i)) blocks = append(blocks, blockID) url += "&comp=block&blockid=" + blockID } else { @@ -164,7 +164,7 @@ func TestActionsArtifactV4UploadSingleFile(t *testing.T) { req = NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/FinalizeArtifact", toProtoJSON(&actions.FinalizeArtifactRequest{ Name: entry.name, Size: int64(entry.append+1) * 1024, - Hash: wrapperspb.String("sha256:" + hex.EncodeToString(sha[:])), + Hash: wrapperspb.String("sha256:" + hex.EncodeToString(sha)), WorkflowRunBackendId: "792", WorkflowJobRunBackendId: "193", })). From 83fe746894fd95a3bf85e835c00f26bea00ca003 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Mon, 2 Mar 2026 21:08:28 +0100 Subject: [PATCH 05/26] Ensure sorting of chunks before extracting it's size * Better error messages for decoding in log --- routers/api/actions/artifactsv4.go | 17 ++- .../api_actions_artifact_v4_test.go | 140 +++++++++--------- 2 files changed, 84 insertions(+), 73 deletions(-) diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 218c6f5c5601a..173ccc365daad 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -95,6 +95,7 @@ import ( "io" "net/http" "net/url" + "sort" "strconv" "strings" "time" @@ -181,10 +182,15 @@ func (r artifactV4Routes) verifySignature(ctx *ArtifactContext, endp string) (*a sig := ctx.Req.URL.Query().Get("sig") expires := ctx.Req.URL.Query().Get("expires") artifactName := ctx.Req.URL.Query().Get("artifactName") - dsig, _ := base64.RawURLEncoding.DecodeString(sig) - taskID, _ := strconv.ParseInt(rawTaskID, 10, 64) - artifactID, _ := strconv.ParseInt(rawArtifactID, 10, 64) - + dsig, errSig := base64.RawURLEncoding.DecodeString(sig) + taskID, errTask := strconv.ParseInt(rawTaskID, 10, 64) + artifactID, errArtifactID := strconv.ParseInt(rawArtifactID, 10, 64) + err := errors.Join(errSig, errTask, errArtifactID) + if err != nil { + log.Error("Error decoding signature values: %v", err) + ctx.HTTPError(http.StatusBadRequest, "Error decoding signature values") + return nil, "", false + } expecedsig := r.buildSignature(endp, expires, artifactName, taskID, artifactID) if !hmac.Equal(dsig, expecedsig) { log.Error("Error unauthorized") @@ -406,6 +412,9 @@ func (r *artifactV4Routes) finalizeArtifact(ctx *ArtifactContext) { ctx.HTTPError(http.StatusInternalServerError, "Error merge chunks") return } + sort.Slice(chunks, func(i, j int) bool { + return chunks[i].Start < chunks[j].Start + }) artifact.FileSize = chunks[len(chunks)-1].End + 1 artifact.FileCompressedSize = chunks[len(chunks)-1].End + 1 diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index 75abcadfd28f9..4127ae91f58cd 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -101,78 +101,80 @@ func TestActionsArtifactV4UploadSingleFile(t *testing.T) { } for _, entry := range table { - // acquire artifact upload url - req := NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/CreateArtifact", toProtoJSON(&actions.CreateArtifactRequest{ - Version: entry.version, - Name: entry.name, - WorkflowRunBackendId: "792", - WorkflowJobRunBackendId: "193", - })).AddTokenAuth(token) - resp := MakeRequest(t, req, http.StatusOK) - var uploadResp actions.CreateArtifactResponse - protojson.Unmarshal(resp.Body.Bytes(), &uploadResp) - assert.True(t, uploadResp.Ok) - assert.Contains(t, uploadResp.SignedUploadUrl, "/twirp/github.actions.results.api.v1.ArtifactService/UploadArtifact") - - h := sha256.New() - - blocks := make([]string, 0, util.Iif(entry.blockID, entry.append+1, 0)) - - // get upload url - idx := strings.Index(uploadResp.SignedUploadUrl, "/twirp/") - for i := range entry.append + 1 { - url := uploadResp.SignedUploadUrl[idx:] - // See https://learn.microsoft.com/en-us/rest/api/storageservices/append-block - // See https://learn.microsoft.com/en-us/rest/api/storageservices/put-block - if entry.blockID { - blockID := base64.RawURLEncoding.EncodeToString(fmt.Append([]byte("SOME_BIG_BLOCK_ID_"), i)) - blocks = append(blocks, blockID) - url += "&comp=block&blockid=" + blockID - } else { - url += "&comp=appendBlock" + t.Run(entry.name, func(t *testing.T) { + // acquire artifact upload url + req := NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/CreateArtifact", toProtoJSON(&actions.CreateArtifactRequest{ + Version: entry.version, + Name: entry.name, + WorkflowRunBackendId: "792", + WorkflowJobRunBackendId: "193", + })).AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + var uploadResp actions.CreateArtifactResponse + protojson.Unmarshal(resp.Body.Bytes(), &uploadResp) + assert.True(t, uploadResp.Ok) + assert.Contains(t, uploadResp.SignedUploadUrl, "/twirp/github.actions.results.api.v1.ArtifactService/UploadArtifact") + + h := sha256.New() + + blocks := make([]string, 0, util.Iif(entry.blockID, entry.append+1, 0)) + + // get upload url + idx := strings.Index(uploadResp.SignedUploadUrl, "/twirp/") + for i := range entry.append + 1 { + url := uploadResp.SignedUploadUrl[idx:] + // See https://learn.microsoft.com/en-us/rest/api/storageservices/append-block + // See https://learn.microsoft.com/en-us/rest/api/storageservices/put-block + if entry.blockID { + blockID := base64.RawURLEncoding.EncodeToString(fmt.Append([]byte("SOME_BIG_BLOCK_ID_"), i)) + blocks = append(blocks, blockID) + url += "&comp=block&blockid=" + blockID + } else { + url += "&comp=appendBlock" + } + + // upload artifact chunk + body := strings.Repeat("A", 1024) + _, _ = h.Write([]byte(body)) + var bodyReader io.Reader = strings.NewReader(body) + if entry.noLength { + bodyReader = io.MultiReader(bodyReader) + } + req = NewRequestWithBody(t, "PUT", url, bodyReader) + MakeRequest(t, req, http.StatusCreated) } - // upload artifact chunk - body := strings.Repeat("A", 1024) - _, _ = h.Write([]byte(body)) - var bodyReader io.Reader = strings.NewReader(body) - if entry.noLength { - bodyReader = io.MultiReader(bodyReader) + if entry.blockID && entry.append > 0 { + // https://learn.microsoft.com/en-us/rest/api/storageservices/put-block-list + blockListURL := uploadResp.SignedUploadUrl[idx:] + "&comp=blocklist" + // upload artifact blockList + blockList := &actions.BlockList{ + Latest: blocks, + } + rawBlockList, err := xml.Marshal(blockList) + assert.NoError(t, err) + req = NewRequestWithBody(t, "PUT", blockListURL, bytes.NewReader(rawBlockList)) + MakeRequest(t, req, http.StatusCreated) } - req = NewRequestWithBody(t, "PUT", url, bodyReader) - MakeRequest(t, req, http.StatusCreated) - } - - if entry.blockID && entry.append > 0 { - // https://learn.microsoft.com/en-us/rest/api/storageservices/put-block-list - blockListURL := uploadResp.SignedUploadUrl[idx:] + "&comp=blocklist" - // upload artifact blockList - blockList := &actions.BlockList{ - Latest: blocks, - } - rawBlockList, err := xml.Marshal(blockList) - assert.NoError(t, err) - req = NewRequestWithBody(t, "PUT", blockListURL, bytes.NewReader(rawBlockList)) - MakeRequest(t, req, http.StatusCreated) - } - - sha := h.Sum(nil) - - t.Logf("Create artifact confirm") - - // confirm artifact upload - req = NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/FinalizeArtifact", toProtoJSON(&actions.FinalizeArtifactRequest{ - Name: entry.name, - Size: int64(entry.append+1) * 1024, - Hash: wrapperspb.String("sha256:" + hex.EncodeToString(sha)), - WorkflowRunBackendId: "792", - WorkflowJobRunBackendId: "193", - })). - AddTokenAuth(token) - resp = MakeRequest(t, req, http.StatusOK) - var finalizeResp actions.FinalizeArtifactResponse - protojson.Unmarshal(resp.Body.Bytes(), &finalizeResp) - assert.True(t, finalizeResp.Ok) + + sha := h.Sum(nil) + + t.Logf("Create artifact confirm") + + // confirm artifact upload + req = NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/FinalizeArtifact", toProtoJSON(&actions.FinalizeArtifactRequest{ + Name: entry.name, + Size: int64(entry.append+1) * 1024, + Hash: wrapperspb.String("sha256:" + hex.EncodeToString(sha)), + WorkflowRunBackendId: "792", + WorkflowJobRunBackendId: "193", + })). + AddTokenAuth(token) + resp = MakeRequest(t, req, http.StatusOK) + var finalizeResp actions.FinalizeArtifactResponse + protojson.Unmarshal(resp.Body.Bytes(), &finalizeResp) + assert.True(t, finalizeResp.Ok) + }) } } From 6b0fe790bea2b838002c9a0b21cd497730181d47 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 3 Mar 2026 18:09:31 +0800 Subject: [PATCH 06/26] refactor --- routers/api/actions/artifacts.go | 8 +-- routers/api/actions/artifacts_chunks.go | 91 ++++++++++++++----------- routers/api/actions/artifacts_utils.go | 7 +- routers/api/actions/artifactsv4.go | 2 +- 4 files changed, 59 insertions(+), 49 deletions(-) diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index e615cfb4e0567..f4dbadbdcf021 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -241,7 +241,7 @@ func (ar artifactRoutes) uploadArtifact(ctx *ArtifactContext) { } // get upload file size - fileRealTotalSize, contentLength := getUploadFileSize(ctx) + fileRealTotalSize := getUploadFileSize(ctx) // get artifact retention days expiredDays := setting.Actions.ArtifactRetentionDays @@ -265,17 +265,17 @@ func (ar artifactRoutes) uploadArtifact(ctx *ArtifactContext) { return } - // save chunk to storage, if success, return chunk stotal size + // save chunk to storage, if success, return chunks total size // if artifact is not gzip when uploading, chunksTotalSize == fileRealTotalSize // if artifact is gzip when uploading, chunksTotalSize < fileRealTotalSize - chunksTotalSize, err := saveUploadChunk(ar.fs, ctx, artifact, contentLength, runID) + chunksTotalSize, err := saveUploadChunkGetTotalSize(ar.fs, ctx, artifact, runID) if err != nil { log.Error("Error save upload chunk: %v", err) ctx.HTTPError(http.StatusInternalServerError, "Error save upload chunk") return } - // update artifact size if zero or not match, over write artifact size + // update artifact size if zero or not match, overwrite artifact size if artifact.FileSize == 0 || artifact.FileCompressedSize == 0 || artifact.FileSize != fileRealTotalSize || diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index c8dc1c3f61263..2e1928194f6a9 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -21,18 +21,29 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/storage" - "code.gitea.io/gitea/modules/util" ) -func saveUploadChunkBase(st storage.ObjectStorage, ctx *ArtifactContext, - artifact *actions.ActionArtifact, - contentSize, runID, start, end, length int64, checkMd5 bool, -) (int64, error) { +type saveUploadChunkOptions struct { + start int64 + end *int64 + checkMd5 bool +} + +func saveUploadChunkBase(st storage.ObjectStorage, ctx *ArtifactContext, artifact *actions.ActionArtifact, + runID int64, opts saveUploadChunkOptions, +) (writtenSize int64, retErr error) { // build chunk store path - storagePath := fmt.Sprintf("tmp%d/%d-%d-%d-%d.chunk", runID, runID, artifact.ID, start, end) + storagePath := fmt.Sprintf("tmp%d/%d-%d-%d.chunk", runID, runID, artifact.ID, opts.start) + + // "end" is optional, so "contentSize=-1" means read until EOF + contentSize := int64(-1) + if opts.end != nil { + contentSize = *opts.end - opts.start + 1 + } + var r io.Reader = ctx.Req.Body var hasher hash.Hash - if checkMd5 { + if opts.checkMd5 { // use io.TeeReader to avoid reading all body to md5 sum. // it writes data to hasher after reading end // if hash is not matched, delete the read-end result @@ -42,55 +53,55 @@ func saveUploadChunkBase(st storage.ObjectStorage, ctx *ArtifactContext, // save chunk to storage writtenSize, err := st.Save(storagePath, r, contentSize) if err != nil { - return -1, fmt.Errorf("save chunk to storage error: %v", err) + return 0, fmt.Errorf("save chunk to storage error: %v", err) + } + + defer func() { + if retErr != nil { + if err := st.Delete(storagePath); err != nil { + log.Error("Error deleting chunk: %s, %v", storagePath, err) + } + } + }() + + if contentSize != -1 && writtenSize != contentSize { + return writtenSize, fmt.Errorf("writtenSize %d does not match contentSize %d", writtenSize, contentSize) } - var checkErr error - if checkMd5 { + if opts.checkMd5 { // check md5 reqMd5String := ctx.Req.Header.Get(artifactXActionsResultsMD5Header) chunkMd5String := base64.StdEncoding.EncodeToString(hasher.Sum(nil)) - log.Info("[artifact] check chunk md5, sum: %s, header: %s", chunkMd5String, reqMd5String) + log.Debug("[artifact] check chunk md5, sum: %s, header: %s", chunkMd5String, reqMd5String) // if md5 not match, delete the chunk if reqMd5String != chunkMd5String { - checkErr = errors.New("md5 not match") + return writtenSize, errors.New("md5 not match") } } - if writtenSize != contentSize && contentSize != -1 { - checkErr = errors.Join(checkErr, fmt.Errorf("writtenSize %d not match contentSize %d", writtenSize, contentSize)) - } - if checkErr != nil { - if err := st.Delete(storagePath); err != nil { - log.Error("Error deleting chunk: %s, %v", storagePath, err) - } - return -1, checkErr - } - log.Info("[artifact] save chunk %s, size: %d, artifact id: %d, start: %d, end: %d", - storagePath, writtenSize, artifact.ID, start, end) - // return chunk total size if length is provided, otherwise return writtenSize - return util.Iif(length != -1, length, writtenSize), nil + log.Debug("[artifact] save chunk %s, size: %d, artifact id: %d, start: %d, size: %d", storagePath, writtenSize, artifact.ID, opts.start, contentSize) + return writtenSize, nil } -func saveUploadChunk(st storage.ObjectStorage, ctx *ArtifactContext, - artifact *actions.ActionArtifact, - contentSize, runID int64, -) (int64, error) { +func saveUploadChunkGetTotalSize(st storage.ObjectStorage, ctx *ArtifactContext, artifact *actions.ActionArtifact, runID int64) (totalSize int64, _ error) { // parse content-range header, format: bytes 0-1023/146515 contentRange := ctx.Req.Header.Get("Content-Range") - start, end, length := int64(0), int64(0), int64(0) - if _, err := fmt.Sscanf(contentRange, "bytes %d-%d/%d", &start, &end, &length); err != nil { - log.Warn("parse content range error: %v, content-range: %s", err, contentRange) - return -1, fmt.Errorf("parse content range error: %v", err) + var start, end int64 + if _, err := fmt.Sscanf(contentRange, "bytes %d-%d/%d", &start, &end, &totalSize); err != nil { + return 0, fmt.Errorf("parse content range error: %v", err) + } + _, err := saveUploadChunkBase(st, ctx, artifact, runID, saveUploadChunkOptions{start: start, end: new(end), checkMd5: true}) + if err != nil { + return 0, err } - return saveUploadChunkBase(st, ctx, artifact, contentSize, runID, start, end, length, true) + return totalSize, nil } // Returns uploaded length -func appendUploadChunk(st storage.ObjectStorage, ctx *ArtifactContext, - artifact *actions.ActionArtifact, - start, contentSize, runID int64, -) (int64, error) { - end := start + contentSize - 1 - return saveUploadChunkBase(st, ctx, artifact, contentSize, runID, start, end, -1, false) +func appendUploadChunk(st storage.ObjectStorage, ctx *ArtifactContext, artifact *actions.ActionArtifact, runID, start int64) (int64, error) { + opts := saveUploadChunkOptions{start: start, checkMd5: true} + if ctx.Req.ContentLength > 0 { + opts.end = new(start + ctx.Req.ContentLength - 1) + } + return saveUploadChunkBase(st, ctx, artifact, runID, opts) } type chunkFileItem struct { diff --git a/routers/api/actions/artifacts_utils.go b/routers/api/actions/artifacts_utils.go index 35868c290e9ef..46e32d726500f 100644 --- a/routers/api/actions/artifacts_utils.go +++ b/routers/api/actions/artifacts_utils.go @@ -84,11 +84,10 @@ func parseArtifactItemPath(ctx *ArtifactContext) (string, string, bool) { // getUploadFileSize returns the size of the file to be uploaded. // The raw size is the size of the file as reported by the header X-TFS-FileLength. -func getUploadFileSize(ctx *ArtifactContext) (int64, int64) { - contentLength := ctx.Req.ContentLength +func getUploadFileSize(ctx *ArtifactContext) int64 { xTfsLength, _ := strconv.ParseInt(ctx.Req.Header.Get(artifactXTfsFileLengthHeader), 10, 64) if xTfsLength > 0 { - return xTfsLength, contentLength + return xTfsLength } - return contentLength, contentLength + return ctx.Req.ContentLength } diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 173ccc365daad..1caeb07dbf80d 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -319,7 +319,7 @@ func (r *artifactV4Routes) uploadArtifact(ctx *ArtifactContext) { } blockid := ctx.Req.URL.Query().Get("blockid") if blockid == "" { - uploadedLength, err := appendUploadChunk(r.fs, ctx, artifact, artifact.FileSize, ctx.Req.ContentLength, artifact.RunID) + uploadedLength, err := appendUploadChunk(r.fs, ctx, artifact, artifact.RunID, artifact.FileSize) if err != nil { log.Error("Error appending Chunk %v", err) ctx.HTTPError(http.StatusInternalServerError, "Error appending Chunk") From 40e62c4776bcebe31b4d9c2b2258e7db96996c7e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 3 Mar 2026 18:47:45 +0800 Subject: [PATCH 07/26] refactor --- routers/api/actions/artifacts_chunks.go | 99 ++++++++++++------------- routers/api/actions/artifactsv4.go | 33 ++++++++- 2 files changed, 80 insertions(+), 52 deletions(-) diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index 2e1928194f6a9..3af55c50b9dea 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -12,7 +12,7 @@ import ( "fmt" "hash" "io" - "path/filepath" + "path" "sort" "strings" "time" @@ -29,11 +29,38 @@ type saveUploadChunkOptions struct { checkMd5 bool } +func makeChunkFilenameV3(runID, artifactID, start int64, endPtr *int64) string { + end := 0 + if endPtr != nil { + end = int(*endPtr) + } + return fmt.Sprintf("%d-%d-%d-%d.chunk", runID, artifactID, start, end) +} + +func parseChunkFileItemV3(st storage.ObjectStorage, storageDir, subPath string) (*chunkFileItem, error) { + var item chunkFileItem + if _, err := fmt.Sscanf(path.Base(subPath), "%d-%d-%d-%d.chunk", &item.RunID, &item.ArtifactID, &item.Start, &item.End); err != nil { + return nil, err + } + + item.Path = storageDir + "/" + subPath + if item.End == 0 { + fi, err := st.Stat(item.Path) + if err != nil { + return nil, err + } + item.End = item.Start + fi.Size() - 1 + } else { + item.Size = item.End - item.Start + 1 + } + return &item, nil +} + func saveUploadChunkBase(st storage.ObjectStorage, ctx *ArtifactContext, artifact *actions.ActionArtifact, runID int64, opts saveUploadChunkOptions, ) (writtenSize int64, retErr error) { // build chunk store path - storagePath := fmt.Sprintf("tmp%d/%d-%d-%d.chunk", runID, runID, artifact.ID, opts.start) + storagePath := fmt.Sprintf("tmp%d/%s.chunk", runID, makeChunkFilenameV3(runID, artifact.ID, opts.start, opts.end)) // "end" is optional, so "contentSize=-1" means read until EOF contentSize := int64(-1) @@ -105,32 +132,26 @@ func appendUploadChunk(st storage.ObjectStorage, ctx *ArtifactContext, artifact } type chunkFileItem struct { - RunID int64 + RunID int64 + Path string + ArtifactID int64 - Start int64 - End int64 - Path string + Size int64 + + Start int64 // v3 only + End int64 // v3 only + ChunkName string // v4 only } func listChunksByRunID(st storage.ObjectStorage, runID int64) (map[int64][]*chunkFileItem, error) { storageDir := fmt.Sprintf("tmp%d", runID) var chunks []*chunkFileItem if err := st.IterateObjects(storageDir, func(fpath string, obj storage.Object) error { - baseName := filepath.Base(fpath) - // when read chunks from storage, it only contains storage dir and basename, - // no matter the subdirectory setting in storage config - item := chunkFileItem{Path: storageDir + "/" + baseName} - if _, err := fmt.Sscanf(baseName, "%d-%d-%d-%d.chunk", &item.RunID, &item.ArtifactID, &item.Start, &item.End); err != nil { - return fmt.Errorf("parse content range error: %v", err) - } - if (item.End + 1 - item.Start) <= 0 { - fi, err := st.Stat(fpath) - if err != nil { - return err - } - item.End = item.Start + fi.Size() - 1 + item, err := parseChunkFileItemV3(st, storageDir, fpath) + if err != nil { + return fmt.Errorf("unable to parse chunk name: %v", fpath) } - chunks = append(chunks, &item) + chunks = append(chunks, item) return nil }); err != nil { return nil, err @@ -155,45 +176,21 @@ func listChunksByRunIDV4(st storage.ObjectStorage, runID, artifactID int64, blis } } if err := st.IterateObjects(storageDir, func(fpath string, obj storage.Object) error { - baseName := filepath.Base(fpath) - if !strings.HasPrefix(baseName, "block-") { - return nil - } - // when read chunks from storage, it only contains storage dir and basename, - // no matter the subdirectory setting in storage config - item := chunkFileItem{Path: storageDir + "/" + baseName, ArtifactID: artifactID} - var size int64 - var b64chunkName string - var fArtifactID int64 - if _, err := fmt.Sscanf(baseName, "block-%d-%d-%d-%s", &item.RunID, &fArtifactID, &size, &b64chunkName); err != nil { - log.Warn("parse content range error: %v", err) - return nil - } - rchunkName, err := base64.URLEncoding.DecodeString(b64chunkName) + item, err := parseChunkFileItemV4(st, storageDir, fpath) if err != nil { - log.Warn("failed to parse chunkName: %v", err) - return nil + return fmt.Errorf("unable to parse chunk name: %v", fpath) } - if fArtifactID != artifactID { + if item.ArtifactID != artifactID { return nil } - if size <= 0 { - fi, err := st.Stat(fpath) - if err != nil { - return err - } - size = fi.Size() - } - chunkName := string(rchunkName) - item.End = item.Start + size - 1 - // Single chunk upload with blockid - if _, ok := chunkMap[chunkName]; ok { - chunkMap[chunkName] = &item + // Single chunk upload with block id + if _, ok := chunkMap[item.ChunkName]; ok { + chunkMap[item.ChunkName] = item } else if chunkMap == nil { if chunks != nil { return errors.New("blockmap is required for chunks > 1") } - chunks = []*chunkFileItem{&item} + chunks = []*chunkFileItem{item} } return nil }); err != nil && (chunks != nil || blist != nil) { diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 1caeb07dbf80d..01261c66a7be3 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -95,6 +95,7 @@ import ( "io" "net/http" "net/url" + "path" "sort" "strconv" "strings" @@ -176,6 +177,34 @@ func (r artifactV4Routes) buildArtifactURL(ctx *ArtifactContext, endp, artifactN return uploadURL } +func makeBlockFilenameV4(runID, artifactID, size int64, blockID string) string { + return fmt.Sprintf("block-%d-%d-%d-%s", runID, artifactID, size, base64.URLEncoding.EncodeToString([]byte(blockID))) +} + +func parseChunkFileItemV4(st storage.ObjectStorage, storageDir, subPath string) (*chunkFileItem, error) { + var item chunkFileItem + var b64chunkName string + _, err := fmt.Sscanf(path.Base(subPath), "block-%d-%d-%d-%s", &item.RunID, &item.ArtifactID, &item.Size, &b64chunkName) + if err != nil { + return nil, err + } + chunkName, err := base64.URLEncoding.DecodeString(b64chunkName) + if err != nil { + return nil, err + } + item.ChunkName = string(chunkName) + item.Path = storageDir + "/" + subPath + if item.Size <= 0 { + fi, err := st.Stat(item.Path) + if err != nil { + return nil, err + } + item.Size = fi.Size() + } + item.End = item.Start + item.End - 1 // FIXME: not right, there is no "item.Start" + return &item, nil +} + func (r artifactV4Routes) verifySignature(ctx *ArtifactContext, endp string) (*actions.ActionTask, string, bool) { rawTaskID := ctx.Req.URL.Query().Get("taskID") rawArtifactID := ctx.Req.URL.Query().Get("artifactID") @@ -333,7 +362,9 @@ func (r *artifactV4Routes) uploadArtifact(ctx *ArtifactContext) { return } } else { - _, err := r.fs.Save(fmt.Sprintf("tmpv4%d/block-%d-%d-%d-%s", task.Job.RunID, task.Job.RunID, artifact.ID, ctx.Req.ContentLength, base64.URLEncoding.EncodeToString([]byte(blockid))), ctx.Req.Body, -1) + // FIXME: why "ctx.Req.ContentLength" is used for filename, but "-1" is used for "Save" + blockFilename := makeBlockFilenameV4(task.Job.RunID, artifact.ID, ctx.Req.ContentLength, blockid) + _, err := r.fs.Save(fmt.Sprintf("tmpv4%d/%s", task.Job.RunID, blockFilename), ctx.Req.Body, -1) if err != nil { log.Error("Error uploading block blob %v", err) ctx.HTTPError(http.StatusInternalServerError, "Error uploading block blob") From f06112174f935357b797a23013ffd5a4296a544f Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 3 Mar 2026 18:59:05 +0800 Subject: [PATCH 08/26] refactor --- routers/api/actions/artifacts_chunks.go | 27 ++++++++++++++++--------- routers/api/actions/artifactsv4.go | 13 ++++++++++-- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index 3af55c50b9dea..6db32a9fa8906 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -38,8 +38,13 @@ func makeChunkFilenameV3(runID, artifactID, start int64, endPtr *int64) string { } func parseChunkFileItemV3(st storage.ObjectStorage, storageDir, subPath string) (*chunkFileItem, error) { + baseName := path.Base(subPath) + if !strings.HasSuffix(baseName, ".chunk") { + return nil, errSkipChunkFile + } + var item chunkFileItem - if _, err := fmt.Sscanf(path.Base(subPath), "%d-%d-%d-%d.chunk", &item.RunID, &item.ArtifactID, &item.Start, &item.End); err != nil { + if _, err := fmt.Sscanf(baseName, "%d-%d-%d-%d.chunk", &item.RunID, &item.ArtifactID, &item.Start, &item.End); err != nil { return nil, err } @@ -49,7 +54,8 @@ func parseChunkFileItemV3(st storage.ObjectStorage, storageDir, subPath string) if err != nil { return nil, err } - item.End = item.Start + fi.Size() - 1 + item.Size = fi.Size() + item.End = item.Start + item.Size - 1 } else { item.Size = item.End - item.Start + 1 } @@ -60,7 +66,7 @@ func saveUploadChunkBase(st storage.ObjectStorage, ctx *ArtifactContext, artifac runID int64, opts saveUploadChunkOptions, ) (writtenSize int64, retErr error) { // build chunk store path - storagePath := fmt.Sprintf("tmp%d/%s.chunk", runID, makeChunkFilenameV3(runID, artifact.ID, opts.start, opts.end)) + storagePath := fmt.Sprintf("tmp%d/%s", runID, makeChunkFilenameV3(runID, artifact.ID, opts.start, opts.end)) // "end" is optional, so "contentSize=-1" means read until EOF contentSize := int64(-1) @@ -148,7 +154,9 @@ func listChunksByRunID(st storage.ObjectStorage, runID int64) (map[int64][]*chun var chunks []*chunkFileItem if err := st.IterateObjects(storageDir, func(fpath string, obj storage.Object) error { item, err := parseChunkFileItemV3(st, storageDir, fpath) - if err != nil { + if errors.Is(err, errSkipChunkFile) { + return nil + } else if err != nil { return fmt.Errorf("unable to parse chunk name: %v", fpath) } chunks = append(chunks, item) @@ -164,6 +172,7 @@ func listChunksByRunID(st storage.ObjectStorage, runID int64) (map[int64][]*chun return chunksMap, nil } +// TODO: move it to artifactsv4.go func listChunksByRunIDV4(st storage.ObjectStorage, runID, artifactID int64, blist *BlockList) ([]*chunkFileItem, error) { storageDir := fmt.Sprintf("tmpv4%d", runID) var chunks []*chunkFileItem @@ -176,13 +185,13 @@ func listChunksByRunIDV4(st storage.ObjectStorage, runID, artifactID int64, blis } } if err := st.IterateObjects(storageDir, func(fpath string, obj storage.Object) error { - item, err := parseChunkFileItemV4(st, storageDir, fpath) - if err != nil { - return fmt.Errorf("unable to parse chunk name: %v", fpath) - } - if item.ArtifactID != artifactID { + item, err := parseChunkFileItemV4(st, artifactID, storageDir, fpath) + if errors.Is(err, errSkipChunkFile) { return nil + } else if err != nil { + return fmt.Errorf("unable to parse chunk name: %v", fpath) } + // Single chunk upload with block id if _, ok := chunkMap[item.ChunkName]; ok { chunkMap[item.ChunkName] = item diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 01261c66a7be3..b1b5d4cead15c 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -181,13 +181,22 @@ func makeBlockFilenameV4(runID, artifactID, size int64, blockID string) string { return fmt.Sprintf("block-%d-%d-%d-%s", runID, artifactID, size, base64.URLEncoding.EncodeToString([]byte(blockID))) } -func parseChunkFileItemV4(st storage.ObjectStorage, storageDir, subPath string) (*chunkFileItem, error) { +var errSkipChunkFile = errors.New("skip this chunk file") + +func parseChunkFileItemV4(st storage.ObjectStorage, artifactID int64, storageDir, subPath string) (*chunkFileItem, error) { + baseName := path.Base(subPath) + if !strings.HasPrefix(baseName, "block-") { + return nil, errSkipChunkFile + } var item chunkFileItem var b64chunkName string - _, err := fmt.Sscanf(path.Base(subPath), "block-%d-%d-%d-%s", &item.RunID, &item.ArtifactID, &item.Size, &b64chunkName) + _, err := fmt.Sscanf(baseName, "block-%d-%d-%d-%s", &item.RunID, &item.ArtifactID, &item.Size, &b64chunkName) if err != nil { return nil, err } + if item.ArtifactID != artifactID { + return nil, errSkipChunkFile + } chunkName, err := base64.URLEncoding.DecodeString(b64chunkName) if err != nil { return nil, err From 0e4e7717e6ea697791d26a3854c0ccb5abb614c8 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 3 Mar 2026 19:05:50 +0800 Subject: [PATCH 09/26] refactor --- routers/api/actions/artifacts.go | 2 +- routers/api/actions/artifacts_chunks.go | 12 ++++++------ routers/api/actions/artifactsv4.go | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index f4dbadbdcf021..6bef6c03c063f 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -268,7 +268,7 @@ func (ar artifactRoutes) uploadArtifact(ctx *ArtifactContext) { // save chunk to storage, if success, return chunks total size // if artifact is not gzip when uploading, chunksTotalSize == fileRealTotalSize // if artifact is gzip when uploading, chunksTotalSize < fileRealTotalSize - chunksTotalSize, err := saveUploadChunkGetTotalSize(ar.fs, ctx, artifact, runID) + chunksTotalSize, err := saveUploadChunkV3GetTotalSize(ar.fs, ctx, artifact, runID) if err != nil { log.Error("Error save upload chunk: %v", err) ctx.HTTPError(http.StatusInternalServerError, "Error save upload chunk") diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index 6db32a9fa8906..d3e7ee96de261 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -62,7 +62,7 @@ func parseChunkFileItemV3(st storage.ObjectStorage, storageDir, subPath string) return &item, nil } -func saveUploadChunkBase(st storage.ObjectStorage, ctx *ArtifactContext, artifact *actions.ActionArtifact, +func saveUploadChunkV3(st storage.ObjectStorage, ctx *ArtifactContext, artifact *actions.ActionArtifact, runID int64, opts saveUploadChunkOptions, ) (writtenSize int64, retErr error) { // build chunk store path @@ -114,14 +114,14 @@ func saveUploadChunkBase(st storage.ObjectStorage, ctx *ArtifactContext, artifac return writtenSize, nil } -func saveUploadChunkGetTotalSize(st storage.ObjectStorage, ctx *ArtifactContext, artifact *actions.ActionArtifact, runID int64) (totalSize int64, _ error) { +func saveUploadChunkV3GetTotalSize(st storage.ObjectStorage, ctx *ArtifactContext, artifact *actions.ActionArtifact, runID int64) (totalSize int64, _ error) { // parse content-range header, format: bytes 0-1023/146515 contentRange := ctx.Req.Header.Get("Content-Range") var start, end int64 if _, err := fmt.Sscanf(contentRange, "bytes %d-%d/%d", &start, &end, &totalSize); err != nil { return 0, fmt.Errorf("parse content range error: %v", err) } - _, err := saveUploadChunkBase(st, ctx, artifact, runID, saveUploadChunkOptions{start: start, end: new(end), checkMd5: true}) + _, err := saveUploadChunkV3(st, ctx, artifact, runID, saveUploadChunkOptions{start: start, end: new(end), checkMd5: true}) if err != nil { return 0, err } @@ -129,12 +129,12 @@ func saveUploadChunkGetTotalSize(st storage.ObjectStorage, ctx *ArtifactContext, } // Returns uploaded length -func appendUploadChunk(st storage.ObjectStorage, ctx *ArtifactContext, artifact *actions.ActionArtifact, runID, start int64) (int64, error) { +func appendUploadChunkV3(st storage.ObjectStorage, ctx *ArtifactContext, artifact *actions.ActionArtifact, runID, start int64) (int64, error) { opts := saveUploadChunkOptions{start: start, checkMd5: true} if ctx.Req.ContentLength > 0 { opts.end = new(start + ctx.Req.ContentLength - 1) } - return saveUploadChunkBase(st, ctx, artifact, runID, opts) + return saveUploadChunkV3(st, ctx, artifact, runID, opts) } type chunkFileItem struct { @@ -205,7 +205,7 @@ func listChunksByRunIDV4(st storage.ObjectStorage, runID, artifactID int64, blis }); err != nil && (chunks != nil || blist != nil) { return nil, err } else if blist == nil && chunks == nil { - var chunkMap map[int64][]*chunkFileItem + var chunkMap map[int64][]*chunkFileItem // FIXME: why it overwrite the chunkMap from parent scope? chunkMap, err = listChunksByRunID(st, runID) if err != nil { return nil, err diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index b1b5d4cead15c..f0ca4911c1cfb 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -357,7 +357,7 @@ func (r *artifactV4Routes) uploadArtifact(ctx *ArtifactContext) { } blockid := ctx.Req.URL.Query().Get("blockid") if blockid == "" { - uploadedLength, err := appendUploadChunk(r.fs, ctx, artifact, artifact.RunID, artifact.FileSize) + uploadedLength, err := appendUploadChunkV3(r.fs, ctx, artifact, artifact.RunID, artifact.FileSize) if err != nil { log.Error("Error appending Chunk %v", err) ctx.HTTPError(http.StatusInternalServerError, "Error appending Chunk") From c6ac5e606baf91a9f124ee10dcff3d6c3a8380cb Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 3 Mar 2026 19:22:43 +0800 Subject: [PATCH 10/26] refactor --- routers/api/actions/artifacts_chunks.go | 17 +++++++++-------- routers/api/actions/artifactsv4.go | 3 ++- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index d3e7ee96de261..0e40ffa410401 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -44,7 +44,8 @@ func parseChunkFileItemV3(st storage.ObjectStorage, storageDir, subPath string) } var item chunkFileItem - if _, err := fmt.Sscanf(baseName, "%d-%d-%d-%d.chunk", &item.RunID, &item.ArtifactID, &item.Start, &item.End); err != nil { + var unusedRunID int64 + if _, err := fmt.Sscanf(baseName, "%d-%d-%d-%d.chunk", &unusedRunID, &item.ArtifactID, &item.Start, &item.End); err != nil { return nil, err } @@ -130,7 +131,7 @@ func saveUploadChunkV3GetTotalSize(st storage.ObjectStorage, ctx *ArtifactContex // Returns uploaded length func appendUploadChunkV3(st storage.ObjectStorage, ctx *ArtifactContext, artifact *actions.ActionArtifact, runID, start int64) (int64, error) { - opts := saveUploadChunkOptions{start: start, checkMd5: true} + opts := saveUploadChunkOptions{start: start} if ctx.Req.ContentLength > 0 { opts.end = new(start + ctx.Req.ContentLength - 1) } @@ -138,14 +139,14 @@ func appendUploadChunkV3(st storage.ObjectStorage, ctx *ArtifactContext, artifac } type chunkFileItem struct { - RunID int64 - Path string - ArtifactID int64 - Size int64 - Start int64 // v3 only - End int64 // v3 only + Path string + Size int64 + + Start int64 // v3 only + End int64 // v3 only + ChunkName string // v4 only } diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index f0ca4911c1cfb..645ec0868e8a5 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -189,8 +189,9 @@ func parseChunkFileItemV4(st storage.ObjectStorage, artifactID int64, storageDir return nil, errSkipChunkFile } var item chunkFileItem + var unusedRunID int64 var b64chunkName string - _, err := fmt.Sscanf(baseName, "block-%d-%d-%d-%s", &item.RunID, &item.ArtifactID, &item.Size, &b64chunkName) + _, err := fmt.Sscanf(baseName, "block-%d-%d-%d-%s", &unusedRunID, &item.ArtifactID, &item.Size, &b64chunkName) if err != nil { return nil, err } From a0829c660223a68fd72ed07147f6dac6e21d9da4 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 3 Mar 2026 20:05:36 +0800 Subject: [PATCH 11/26] refactor --- modules/storage/local.go | 2 +- modules/storage/storage.go | 7 ++++++- routers/api/actions/artifacts_chunks.go | 10 +++++----- routers/api/actions/artifactsv4.go | 6 +++--- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/modules/storage/local.go b/modules/storage/local.go index 8a1776f606db1..364f4e26ae66d 100644 --- a/modules/storage/local.go +++ b/modules/storage/local.go @@ -145,7 +145,7 @@ func (l *LocalStorage) IterateObjects(dirName string, fn func(path string, obj O return err } defer obj.Close() - return fn(relPath, obj) + return fn(filepath.ToSlash(relPath), obj) }) } diff --git a/modules/storage/storage.go b/modules/storage/storage.go index 1868817c057cf..3ead05a669539 100644 --- a/modules/storage/storage.go +++ b/modules/storage/storage.go @@ -68,7 +68,12 @@ type ObjectStorage interface { Stat(path string) (os.FileInfo, error) Delete(path string) error URL(path, name, method string, reqParams url.Values) (*url.URL, error) - IterateObjects(path string, iterator func(path string, obj Object) error) error + + // IterateObjects calls the iterator function for each object in the storage with the given path as prefix + // The "fullPath" in is the full path in this storage. + // * IterateObjects("", ...): iterate all objects in this storage + // * IterateObjects("sub-path", ...): iterate all objects with "sub-path" as prefix in this storage, the "fullPath" will be like "sub-path/xxx" + IterateObjects(basePath string, iterator func(fullPath string, obj Object) error) error } // Copy copies a file from source ObjectStorage to dest ObjectStorage diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index 0e40ffa410401..54d88046033d5 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -37,8 +37,8 @@ func makeChunkFilenameV3(runID, artifactID, start int64, endPtr *int64) string { return fmt.Sprintf("%d-%d-%d-%d.chunk", runID, artifactID, start, end) } -func parseChunkFileItemV3(st storage.ObjectStorage, storageDir, subPath string) (*chunkFileItem, error) { - baseName := path.Base(subPath) +func parseChunkFileItemV3(st storage.ObjectStorage, fpath string) (*chunkFileItem, error) { + baseName := path.Base(fpath) if !strings.HasSuffix(baseName, ".chunk") { return nil, errSkipChunkFile } @@ -49,7 +49,7 @@ func parseChunkFileItemV3(st storage.ObjectStorage, storageDir, subPath string) return nil, err } - item.Path = storageDir + "/" + subPath + item.Path = fpath if item.End == 0 { fi, err := st.Stat(item.Path) if err != nil { @@ -154,7 +154,7 @@ func listChunksByRunID(st storage.ObjectStorage, runID int64) (map[int64][]*chun storageDir := fmt.Sprintf("tmp%d", runID) var chunks []*chunkFileItem if err := st.IterateObjects(storageDir, func(fpath string, obj storage.Object) error { - item, err := parseChunkFileItemV3(st, storageDir, fpath) + item, err := parseChunkFileItemV3(st, fpath) if errors.Is(err, errSkipChunkFile) { return nil } else if err != nil { @@ -186,7 +186,7 @@ func listChunksByRunIDV4(st storage.ObjectStorage, runID, artifactID int64, blis } } if err := st.IterateObjects(storageDir, func(fpath string, obj storage.Object) error { - item, err := parseChunkFileItemV4(st, artifactID, storageDir, fpath) + item, err := parseChunkFileItemV4(st, artifactID, fpath) if errors.Is(err, errSkipChunkFile) { return nil } else if err != nil { diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 645ec0868e8a5..1ff9d73fce83f 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -183,8 +183,8 @@ func makeBlockFilenameV4(runID, artifactID, size int64, blockID string) string { var errSkipChunkFile = errors.New("skip this chunk file") -func parseChunkFileItemV4(st storage.ObjectStorage, artifactID int64, storageDir, subPath string) (*chunkFileItem, error) { - baseName := path.Base(subPath) +func parseChunkFileItemV4(st storage.ObjectStorage, artifactID int64, fpath string) (*chunkFileItem, error) { + baseName := path.Base(fpath) if !strings.HasPrefix(baseName, "block-") { return nil, errSkipChunkFile } @@ -203,7 +203,7 @@ func parseChunkFileItemV4(st storage.ObjectStorage, artifactID int64, storageDir return nil, err } item.ChunkName = string(chunkName) - item.Path = storageDir + "/" + subPath + item.Path = fpath if item.Size <= 0 { fi, err := st.Stat(item.Path) if err != nil { From 190070f5a9b2c053accd5c3bb023524c43c3dec1 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 3 Mar 2026 20:55:22 +0800 Subject: [PATCH 12/26] fix --- routers/api/actions/artifactsv4.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 1ff9d73fce83f..bc6dd56d77d39 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -96,7 +96,6 @@ import ( "net/http" "net/url" "path" - "sort" "strconv" "strings" "time" @@ -211,7 +210,6 @@ func parseChunkFileItemV4(st storage.ObjectStorage, artifactID int64, fpath stri } item.Size = fi.Size() } - item.End = item.Start + item.End - 1 // FIXME: not right, there is no "item.Start" return &item, nil } @@ -449,15 +447,16 @@ func (r *artifactV4Routes) finalizeArtifact(ctx *ArtifactContext) { blockList, blockListErr := r.readBlockList(runID, artifact.ID) chunks, err = listChunksByRunIDV4(r.fs, runID, artifact.ID, blockList) if err != nil { - log.Error("Error merge chunks: %v", errors.Join(blockListErr, err)) - ctx.HTTPError(http.StatusInternalServerError, "Error merge chunks") + log.Error("Error list chunks: %v", errors.Join(blockListErr, err)) + ctx.HTTPError(http.StatusInternalServerError, "Error list chunks") return } - sort.Slice(chunks, func(i, j int) bool { - return chunks[i].Start < chunks[j].Start - }) - artifact.FileSize = chunks[len(chunks)-1].End + 1 - artifact.FileCompressedSize = chunks[len(chunks)-1].End + 1 + mergedFileSize := int64(0) + for _, chunk := range chunks { + mergedFileSize += chunk.Size + } + artifact.FileSize = mergedFileSize + artifact.FileCompressedSize = mergedFileSize if req.Size != artifact.FileSize { log.Error("Error merge chunks size mismatch") From 1044664a968a0299c97be3db8b22c06c615b3295 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 3 Mar 2026 21:06:52 +0800 Subject: [PATCH 13/26] refactor --- routers/api/actions/artifacts_chunks.go | 12 +++---- routers/api/actions/artifactsv4.go | 42 ++++++++++++++----------- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index 54d88046033d5..baf893d3ad446 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -300,12 +300,12 @@ func mergeChunksForArtifact(ctx *ArtifactContext, chunks []*chunkFileItem, st st } mergedReader := io.MultiReader(readers...) shaPrefix := "sha256:" - var hash hash.Hash + var hashMd5 hash.Hash if strings.HasPrefix(checksum, shaPrefix) { - hash = sha256.New() + hashMd5 = sha256.New() } - if hash != nil { - mergedReader = io.TeeReader(mergedReader, hash) + if hashMd5 != nil { + mergedReader = io.TeeReader(mergedReader, hashMd5) } // if chunk is gzip, use gz as extension @@ -335,8 +335,8 @@ func mergeChunksForArtifact(ctx *ArtifactContext, chunks []*chunkFileItem, st st } }() - if hash != nil { - rawChecksum := hash.Sum(nil) + if hashMd5 != nil { + rawChecksum := hashMd5.Sum(nil) actualChecksum := hex.EncodeToString(rawChecksum) if !strings.HasSuffix(checksum, actualChecksum) { return fmt.Errorf("update artifact error checksum is invalid %v vs %v", checksum, actualChecksum) diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index bc6dd56d77d39..b6b125169dfd4 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -111,7 +111,7 @@ import ( "code.gitea.io/gitea/services/context" "google.golang.org/protobuf/encoding/protojson" - protoreflect "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -159,25 +159,31 @@ func ArtifactsV4Routes(prefix string) *web.Router { return m } -func (r artifactV4Routes) buildSignature(endp, expires, artifactName string, taskID, artifactID int64) []byte { +func (r *artifactV4Routes) buildSignature(endpoint, expires, artifactName string, taskID, artifactID int64) []byte { mac := hmac.New(sha256.New, setting.GetGeneralTokenSigningSecret()) - mac.Write([]byte(endp)) + mac.Write([]byte(endpoint)) mac.Write([]byte(expires)) mac.Write([]byte(artifactName)) - fmt.Fprint(mac, taskID) - fmt.Fprint(mac, artifactID) + _, _ = fmt.Fprint(mac, taskID) + _, _ = fmt.Fprint(mac, artifactID) return mac.Sum(nil) } -func (r artifactV4Routes) buildArtifactURL(ctx *ArtifactContext, endp, artifactName string, taskID, artifactID int64) string { +func (r *artifactV4Routes) buildArtifactURL(ctx *ArtifactContext, endpoint, artifactName string, taskID, artifactID int64) string { expires := time.Now().Add(60 * time.Minute).Format("2006-01-02 15:04:05.999999999 -0700 MST") uploadURL := strings.TrimSuffix(httplib.GuessCurrentAppURL(ctx), "/") + strings.TrimSuffix(r.prefix, "/") + - "/" + endp + "?sig=" + base64.RawURLEncoding.EncodeToString(r.buildSignature(endp, expires, artifactName, taskID, artifactID)) + "&expires=" + url.QueryEscape(expires) + "&artifactName=" + url.QueryEscape(artifactName) + "&taskID=" + strconv.FormatInt(taskID, 10) + "&artifactID=" + strconv.FormatInt(artifactID, 10) + "/" + endpoint + + "?sig=" + base64.RawURLEncoding.EncodeToString(r.buildSignature(endpoint, expires, artifactName, taskID, artifactID)) + + "&expires=" + url.QueryEscape(expires) + + "&artifactName=" + url.QueryEscape(artifactName) + + "&taskID=" + strconv.FormatInt(taskID, 10) + + "&artifactID=" + strconv.FormatInt(artifactID, 10) return uploadURL } func makeBlockFilenameV4(runID, artifactID, size int64, blockID string) string { - return fmt.Sprintf("block-%d-%d-%d-%s", runID, artifactID, size, base64.URLEncoding.EncodeToString([]byte(blockID))) + sizeInName := max(size, 0) // do not use "-1" in filename + return fmt.Sprintf("block-%d-%d-%d-%s", runID, artifactID, sizeInName, base64.URLEncoding.EncodeToString([]byte(blockID))) } var errSkipChunkFile = errors.New("skip this chunk file") @@ -213,7 +219,7 @@ func parseChunkFileItemV4(st storage.ObjectStorage, artifactID int64, fpath stri return &item, nil } -func (r artifactV4Routes) verifySignature(ctx *ArtifactContext, endp string) (*actions.ActionTask, string, bool) { +func (r *artifactV4Routes) verifySignature(ctx *ArtifactContext, endp string) (*actions.ActionTask, string, bool) { rawTaskID := ctx.Req.URL.Query().Get("taskID") rawArtifactID := ctx.Req.URL.Query().Get("artifactID") sig := ctx.Req.URL.Query().Get("sig") @@ -286,7 +292,7 @@ func (r *artifactV4Routes) parseProtbufBody(ctx *ArtifactContext, req protorefle return true } -func (r *artifactV4Routes) sendProtbufBody(ctx *ArtifactContext, req protoreflect.ProtoMessage) { +func (r *artifactV4Routes) sendProtobufBody(ctx *ArtifactContext, req protoreflect.ProtoMessage) { resp, err := protojson.Marshal(req) if err != nil { log.Error("Error encode response body: %v", err) @@ -335,7 +341,7 @@ func (r *artifactV4Routes) createArtifact(ctx *ArtifactContext) { Ok: true, SignedUploadUrl: r.buildArtifactURL(ctx, "UploadArtifact", artifactName, ctx.ActionTask.ID, artifact.ID), } - r.sendProtbufBody(ctx, &respData) + r.sendProtobufBody(ctx, &respData) } func (r *artifactV4Routes) uploadArtifact(ctx *ArtifactContext) { @@ -354,8 +360,8 @@ func (r *artifactV4Routes) uploadArtifact(ctx *ArtifactContext) { ctx.HTTPError(http.StatusNotFound, "Error artifact not found") return } - blockid := ctx.Req.URL.Query().Get("blockid") - if blockid == "" { + blockID := ctx.Req.URL.Query().Get("blockid") + if blockID == "" { uploadedLength, err := appendUploadChunkV3(r.fs, ctx, artifact, artifact.RunID, artifact.FileSize) if err != nil { log.Error("Error appending Chunk %v", err) @@ -371,7 +377,7 @@ func (r *artifactV4Routes) uploadArtifact(ctx *ArtifactContext) { } } else { // FIXME: why "ctx.Req.ContentLength" is used for filename, but "-1" is used for "Save" - blockFilename := makeBlockFilenameV4(task.Job.RunID, artifact.ID, ctx.Req.ContentLength, blockid) + blockFilename := makeBlockFilenameV4(task.Job.RunID, artifact.ID, ctx.Req.ContentLength, blockID) _, err := r.fs.Save(fmt.Sprintf("tmpv4%d/%s", task.Job.RunID, blockFilename), ctx.Req.Body, -1) if err != nil { log.Error("Error uploading block blob %v", err) @@ -478,7 +484,7 @@ func (r *artifactV4Routes) finalizeArtifact(ctx *ArtifactContext) { Ok: true, ArtifactId: artifact.ID, } - r.sendProtbufBody(ctx, &respData) + r.sendProtobufBody(ctx, &respData) } func (r *artifactV4Routes) listArtifacts(ctx *ArtifactContext) { @@ -529,7 +535,7 @@ func (r *artifactV4Routes) listArtifacts(ctx *ArtifactContext) { respData := ListArtifactsResponse{ Artifacts: list, } - r.sendProtbufBody(ctx, &respData) + r.sendProtobufBody(ctx, &respData) } func (r *artifactV4Routes) getSignedArtifactURL(ctx *ArtifactContext) { @@ -569,7 +575,7 @@ func (r *artifactV4Routes) getSignedArtifactURL(ctx *ArtifactContext) { if respData.SignedUrl == "" { respData.SignedUrl = r.buildArtifactURL(ctx, "DownloadArtifact", artifactName, ctx.ActionTask.ID, artifact.ID) } - r.sendProtbufBody(ctx, &respData) + r.sendProtobufBody(ctx, &respData) } func (r *artifactV4Routes) downloadArtifact(ctx *ArtifactContext) { @@ -626,5 +632,5 @@ func (r *artifactV4Routes) deleteArtifact(ctx *ArtifactContext) { Ok: true, ArtifactId: artifact.ID, } - r.sendProtbufBody(ctx, &respData) + r.sendProtobufBody(ctx, &respData) } From 6b1ee3b956bae203e2fd9b864cc34b35f35b0d93 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 3 Mar 2026 21:11:56 +0800 Subject: [PATCH 14/26] refactor --- routers/api/actions/artifacts_utils.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/routers/api/actions/artifacts_utils.go b/routers/api/actions/artifacts_utils.go index 46e32d726500f..db62f0061b99a 100644 --- a/routers/api/actions/artifacts_utils.go +++ b/routers/api/actions/artifacts_utils.go @@ -20,8 +20,8 @@ const ( artifactXActionsResultsMD5Header = "x-actions-results-md5" ) -// The rules are from https://github.com/actions/toolkit/blob/main/packages/artifact/src/internal/path-and-artifact-name-validation.ts#L32 -var invalidArtifactNameChars = strings.Join([]string{"\\", "/", "\"", ":", "<", ">", "|", "*", "?", "\r", "\n"}, "") +// The rules are from https://github.com/actions/toolkit/blob/main/packages/artifact/src/internal/upload/path-and-artifact-name-validation.ts +const invalidArtifactNameChars = "\\/\":<>|*?\r\n" func validateArtifactName(ctx *ArtifactContext, artifactName string) bool { if strings.ContainsAny(artifactName, invalidArtifactNameChars) { From e3a2ffa491f7e8200161d3e21f2ae2d1558f333c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 3 Mar 2026 21:37:57 +0800 Subject: [PATCH 15/26] refactor --- routers/api/actions/artifacts_chunks.go | 75 +++++++++++++++---------- routers/api/actions/artifactsv4.go | 10 +--- 2 files changed, 47 insertions(+), 38 deletions(-) diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index baf893d3ad446..d17015c45b395 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -141,16 +141,15 @@ func appendUploadChunkV3(st storage.ObjectStorage, ctx *ArtifactContext, artifac type chunkFileItem struct { ArtifactID int64 - Path string - Size int64 - - Start int64 // v3 only - End int64 // v3 only + Path string + Size int64 + Start int64 + End int64 // inclusive ChunkName string // v4 only } -func listChunksByRunID(st storage.ObjectStorage, runID int64) (map[int64][]*chunkFileItem, error) { +func listV3UnorderedChunksMapByRunID(st storage.ObjectStorage, runID int64) (map[int64][]*chunkFileItem, error) { storageDir := fmt.Sprintf("tmp%d", runID) var chunks []*chunkFileItem if err := st.IterateObjects(storageDir, func(fpath string, obj storage.Object) error { @@ -173,18 +172,25 @@ func listChunksByRunID(st storage.ObjectStorage, runID int64) (map[int64][]*chun return chunksMap, nil } -// TODO: move it to artifactsv4.go -func listChunksByRunIDV4(st storage.ObjectStorage, runID, artifactID int64, blist *BlockList) ([]*chunkFileItem, error) { +func listOrderedChunksByRunID(st storage.ObjectStorage, runID, artifactID int64, blist *BlockList) ([]*chunkFileItem, error) { + emptyListAsError := func(chunks []*chunkFileItem) ([]*chunkFileItem, error) { + if len(chunks) == 0 { + return nil, fmt.Errorf("no chunk found for artifact id: %d", artifactID) + } + return chunks, nil + } + storageDir := fmt.Sprintf("tmpv4%d", runID) var chunks []*chunkFileItem - var chunkMap map[string]*chunkFileItem + var chunkMapV4 map[string]*chunkFileItem + if blist != nil { - chunkMap = map[string]*chunkFileItem{} - dummy := &chunkFileItem{} + chunkMapV4 = map[string]*chunkFileItem{} for _, name := range blist.Latest { - chunkMap[name] = dummy + chunkMapV4[name] = nil } } + if err := st.IterateObjects(storageDir, func(fpath string, obj storage.Object) error { item, err := parseChunkFileItemV4(st, artifactID, fpath) if errors.Is(err, errSkipChunkFile) { @@ -194,42 +200,49 @@ func listChunksByRunIDV4(st storage.ObjectStorage, runID, artifactID int64, blis } // Single chunk upload with block id - if _, ok := chunkMap[item.ChunkName]; ok { - chunkMap[item.ChunkName] = item - } else if chunkMap == nil { + if _, ok := chunkMapV4[item.ChunkName]; ok { + chunkMapV4[item.ChunkName] = item + } else if chunkMapV4 == nil { if chunks != nil { return errors.New("blockmap is required for chunks > 1") } chunks = []*chunkFileItem{item} } return nil - }); err != nil && (chunks != nil || blist != nil) { + }); err != nil { return nil, err - } else if blist == nil && chunks == nil { - var chunkMap map[int64][]*chunkFileItem // FIXME: why it overwrite the chunkMap from parent scope? - chunkMap, err = listChunksByRunID(st, runID) + } + + if blist == nil && chunks == nil { + chunkUnorderedItemsMapV3, err := listV3UnorderedChunksMapByRunID(st, runID) if err != nil { return nil, err } - chunks = chunkMap[artifactID] + chunks = chunkUnorderedItemsMapV3[artifactID] + sort.Slice(chunks, func(i, j int) bool { + return chunks[i].Start < chunks[j].Start + }) + return emptyListAsError(chunks) } - if blist != nil { + + if len(chunks) == 0 && blist != nil { for i, name := range blist.Latest { - chunk, ok := chunkMap[name] + chunk, ok := chunkMapV4[name] if !ok || chunk.Path == "" { return nil, fmt.Errorf("missing Chunk (%d/%d): %s", i, len(blist.Latest), name) } chunks = append(chunks, chunk) - if i > 0 { - chunk.Start = chunkMap[blist.Latest[i-1]].End + 1 - chunk.End += chunk.Start - } } } - if len(chunks) < 1 { - return nil, errors.New("no Chunk found") + for i, chunk := range chunks { + if i == 0 { + chunk.End += chunk.Size - 1 + } else { + chunk.Start = chunkMapV4[blist.Latest[i-1]].End + 1 + chunk.End = chunk.Start + chunk.Size - 1 + } } - return chunks, nil + return emptyListAsError(chunks) } func mergeChunksForRun(ctx *ArtifactContext, st storage.ObjectStorage, runID int64, artifactName string) error { @@ -242,13 +255,13 @@ func mergeChunksForRun(ctx *ArtifactContext, st storage.ObjectStorage, runID int return err } // read all uploading chunks from storage - chunksMap, err := listChunksByRunID(st, runID) + unorderedChunksMap, err := listV3UnorderedChunksMapByRunID(st, runID) if err != nil { return err } // range db artifacts to merge chunks for _, art := range artifacts { - chunks, ok := chunksMap[art.ID] + chunks, ok := unorderedChunksMap[art.ID] if !ok { log.Debug("artifact %d chunks not found", art.ID) continue diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index b6b125169dfd4..b7375495d5399 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -451,18 +451,14 @@ func (r *artifactV4Routes) finalizeArtifact(ctx *ArtifactContext) { var chunks []*chunkFileItem blockList, blockListErr := r.readBlockList(runID, artifact.ID) - chunks, err = listChunksByRunIDV4(r.fs, runID, artifact.ID, blockList) + chunks, err = listOrderedChunksByRunID(r.fs, runID, artifact.ID, blockList) if err != nil { log.Error("Error list chunks: %v", errors.Join(blockListErr, err)) ctx.HTTPError(http.StatusInternalServerError, "Error list chunks") return } - mergedFileSize := int64(0) - for _, chunk := range chunks { - mergedFileSize += chunk.Size - } - artifact.FileSize = mergedFileSize - artifact.FileCompressedSize = mergedFileSize + artifact.FileSize = chunks[len(chunks)-1].End + 1 + artifact.FileCompressedSize = chunks[len(chunks)-1].End + 1 if req.Size != artifact.FileSize { log.Error("Error merge chunks size mismatch") From 42d6e56f776ded642e759b76deaa80f3d8a575f1 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 3 Mar 2026 21:51:01 +0800 Subject: [PATCH 16/26] fix typo --- routers/api/actions/artifactsv4.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index b7375495d5399..036d0bf4e39b2 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -276,7 +276,7 @@ func (r *artifactV4Routes) getArtifactByName(ctx *ArtifactContext, runID int64, return &art, nil } -func (r *artifactV4Routes) parseProtbufBody(ctx *ArtifactContext, req protoreflect.ProtoMessage) bool { +func (r *artifactV4Routes) parseProtobufBody(ctx *ArtifactContext, req protoreflect.ProtoMessage) bool { body, err := io.ReadAll(ctx.Req.Body) if err != nil { log.Error("Error decode request body: %v", err) @@ -307,7 +307,7 @@ func (r *artifactV4Routes) sendProtobufBody(ctx *ArtifactContext, req protorefle func (r *artifactV4Routes) createArtifact(ctx *ArtifactContext) { var req CreateArtifactRequest - if ok := r.parseProtbufBody(ctx, &req); !ok { + if ok := r.parseProtobufBody(ctx, &req); !ok { return } _, _, ok := validateRunIDV4(ctx, req.WorkflowRunBackendId) @@ -433,7 +433,7 @@ func (r *artifactV4Routes) readBlockList(runID, artifactID int64) (*BlockList, e func (r *artifactV4Routes) finalizeArtifact(ctx *ArtifactContext) { var req FinalizeArtifactRequest - if ok := r.parseProtbufBody(ctx, &req); !ok { + if ok := r.parseProtobufBody(ctx, &req); !ok { return } _, runID, ok := validateRunIDV4(ctx, req.WorkflowRunBackendId) @@ -486,7 +486,7 @@ func (r *artifactV4Routes) finalizeArtifact(ctx *ArtifactContext) { func (r *artifactV4Routes) listArtifacts(ctx *ArtifactContext) { var req ListArtifactsRequest - if ok := r.parseProtbufBody(ctx, &req); !ok { + if ok := r.parseProtobufBody(ctx, &req); !ok { return } _, runID, ok := validateRunIDV4(ctx, req.WorkflowRunBackendId) @@ -537,7 +537,7 @@ func (r *artifactV4Routes) listArtifacts(ctx *ArtifactContext) { func (r *artifactV4Routes) getSignedArtifactURL(ctx *ArtifactContext) { var req GetSignedArtifactURLRequest - if ok := r.parseProtbufBody(ctx, &req); !ok { + if ok := r.parseProtobufBody(ctx, &req); !ok { return } _, runID, ok := validateRunIDV4(ctx, req.WorkflowRunBackendId) @@ -601,7 +601,7 @@ func (r *artifactV4Routes) downloadArtifact(ctx *ArtifactContext) { func (r *artifactV4Routes) deleteArtifact(ctx *ArtifactContext) { var req DeleteArtifactRequest - if ok := r.parseProtbufBody(ctx, &req); !ok { + if ok := r.parseProtobufBody(ctx, &req); !ok { return } _, runID, ok := validateRunIDV4(ctx, req.WorkflowRunBackendId) From ddf93c67d61afb3c376bf8f9c30e372f8115fa59 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 3 Mar 2026 21:54:32 +0800 Subject: [PATCH 17/26] fine tune --- routers/api/actions/artifacts_chunks.go | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index d17015c45b395..af4098d421d89 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -30,9 +30,9 @@ type saveUploadChunkOptions struct { } func makeChunkFilenameV3(runID, artifactID, start int64, endPtr *int64) string { - end := 0 + var end int64 if endPtr != nil { - end = int(*endPtr) + end = *endPtr } return fmt.Sprintf("%d-%d-%d-%d.chunk", runID, artifactID, start, end) } @@ -140,11 +140,12 @@ func appendUploadChunkV3(st storage.ObjectStorage, ctx *ArtifactContext, artifac type chunkFileItem struct { ArtifactID int64 + Path string - Path string + // these offset/size related fields might be missing when parsing, they will be filled in the listing functions Size int64 Start int64 - End int64 // inclusive + End int64 // inclusive: Size=10, Start=0, End=9 ChunkName string // v4 only } @@ -313,12 +314,14 @@ func mergeChunksForArtifact(ctx *ArtifactContext, chunks []*chunkFileItem, st st } mergedReader := io.MultiReader(readers...) shaPrefix := "sha256:" - var hashMd5 hash.Hash + var hashSha256 hash.Hash if strings.HasPrefix(checksum, shaPrefix) { - hashMd5 = sha256.New() + hashSha256 = sha256.New() + } else if checksum != "" { + log.Error("unsupported checksum format: %s, will skip the checksum verification", checksum) } - if hashMd5 != nil { - mergedReader = io.TeeReader(mergedReader, hashMd5) + if hashSha256 != nil { + mergedReader = io.TeeReader(mergedReader, hashSha256) } // if chunk is gzip, use gz as extension @@ -348,8 +351,8 @@ func mergeChunksForArtifact(ctx *ArtifactContext, chunks []*chunkFileItem, st st } }() - if hashMd5 != nil { - rawChecksum := hashMd5.Sum(nil) + if hashSha256 != nil { + rawChecksum := hashSha256.Sum(nil) actualChecksum := hex.EncodeToString(rawChecksum) if !strings.HasSuffix(checksum, actualChecksum) { return fmt.Errorf("update artifact error checksum is invalid %v vs %v", checksum, actualChecksum) From 78292c713fbdb17c0847d95bb92a5d67dc1f60ac Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 3 Mar 2026 22:38:12 +0800 Subject: [PATCH 18/26] fix local storage --- modules/storage/local.go | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/modules/storage/local.go b/modules/storage/local.go index 364f4e26ae66d..b534378be1ccb 100644 --- a/modules/storage/local.go +++ b/modules/storage/local.go @@ -5,6 +5,7 @@ package storage import ( "context" + "errors" "fmt" "io" "net/url" @@ -118,31 +119,35 @@ func (l *LocalStorage) URL(path, name, _ string, reqParams url.Values) (*url.URL return nil, ErrURLNotSupported } +func (l *LocalStorage) normalizeWalkError(err error) error { + if errors.Is(err, os.ErrNotExist) { + // ignore it because the file may be deleted during the walk, and we don't care about it + return nil + } + return err +} + // IterateObjects iterates across the objects in the local storage func (l *LocalStorage) IterateObjects(dirName string, fn func(path string, obj Object) error) error { dir := l.buildLocalPath(dirName) - return filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error { - if err != nil { + return filepath.WalkDir(dir, func(path string, d os.DirEntry, errWalk error) error { + if err := l.ctx.Err(); err != nil { return err } - select { - case <-l.ctx.Done(): - return l.ctx.Err() - default: + if errWalk != nil { + return l.normalizeWalkError(errWalk) } - if path == l.dir { - return nil - } - if d.IsDir() { + if path == l.dir || d.IsDir() { return nil } + relPath, err := filepath.Rel(l.dir, path) if err != nil { - return err + return l.normalizeWalkError(err) } obj, err := os.Open(path) if err != nil { - return err + return l.normalizeWalkError(err) } defer obj.Close() return fn(filepath.ToSlash(relPath), obj) From aa05af6110cee6326ea513b665689268fbf857b5 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Tue, 3 Mar 2026 20:51:09 +0100 Subject: [PATCH 19/26] Use ctx.Req.ContentLength for r.fs.Save * specifing -1 does not make sense at all --- routers/api/actions/artifactsv4.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 036d0bf4e39b2..4d458a750071d 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -376,9 +376,8 @@ func (r *artifactV4Routes) uploadArtifact(ctx *ArtifactContext) { return } } else { - // FIXME: why "ctx.Req.ContentLength" is used for filename, but "-1" is used for "Save" blockFilename := makeBlockFilenameV4(task.Job.RunID, artifact.ID, ctx.Req.ContentLength, blockID) - _, err := r.fs.Save(fmt.Sprintf("tmpv4%d/%s", task.Job.RunID, blockFilename), ctx.Req.Body, -1) + _, err := r.fs.Save(fmt.Sprintf("tmpv4%d/%s", task.Job.RunID, blockFilename), ctx.Req.Body, ctx.Req.ContentLength) if err != nil { log.Error("Error uploading block blob %v", err) ctx.HTTPError(http.StatusInternalServerError, "Error uploading block blob") From 78cf171f00df3f6d011fa9ff9eac11daebe7e0c5 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 4 Mar 2026 07:19:37 +0800 Subject: [PATCH 20/26] fine tune name & log --- routers/api/actions/artifacts_chunks.go | 7 ++++--- routers/api/actions/artifactsv4.go | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index af4098d421d89..56243257dbeb7 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -20,6 +20,7 @@ import ( "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" ) @@ -173,7 +174,7 @@ func listV3UnorderedChunksMapByRunID(st storage.ObjectStorage, runID int64) (map return chunksMap, nil } -func listOrderedChunksByRunID(st storage.ObjectStorage, runID, artifactID int64, blist *BlockList) ([]*chunkFileItem, error) { +func listOrderedChunksForArtifact(st storage.ObjectStorage, runID, artifactID int64, blist *BlockList) ([]*chunkFileItem, error) { emptyListAsError := func(chunks []*chunkFileItem) ([]*chunkFileItem, error) { if len(chunks) == 0 { return nil, fmt.Errorf("no chunk found for artifact id: %d", artifactID) @@ -230,7 +231,7 @@ func listOrderedChunksByRunID(st storage.ObjectStorage, runID, artifactID int64, for i, name := range blist.Latest { chunk, ok := chunkMapV4[name] if !ok || chunk.Path == "" { - return nil, fmt.Errorf("missing Chunk (%d/%d): %s", i, len(blist.Latest), name) + return nil, fmt.Errorf("missing chunk (%d/%d): %s", i, len(blist.Latest), name) } chunks = append(chunks, chunk) } @@ -318,7 +319,7 @@ func mergeChunksForArtifact(ctx *ArtifactContext, chunks []*chunkFileItem, st st if strings.HasPrefix(checksum, shaPrefix) { hashSha256 = sha256.New() } else if checksum != "" { - log.Error("unsupported checksum format: %s, will skip the checksum verification", checksum) + setting.PanicInDevOrTesting("unsupported checksum format: %s, will skip the checksum verification", checksum) } if hashSha256 != nil { mergedReader = io.TeeReader(mergedReader, hashSha256) diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 4d458a750071d..108d5a6bb0d03 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -364,7 +364,7 @@ func (r *artifactV4Routes) uploadArtifact(ctx *ArtifactContext) { if blockID == "" { uploadedLength, err := appendUploadChunkV3(r.fs, ctx, artifact, artifact.RunID, artifact.FileSize) if err != nil { - log.Error("Error appending Chunk %v", err) + log.Error("Error appending chunk %v", err) ctx.HTTPError(http.StatusInternalServerError, "Error appending Chunk") return } @@ -450,7 +450,7 @@ func (r *artifactV4Routes) finalizeArtifact(ctx *ArtifactContext) { var chunks []*chunkFileItem blockList, blockListErr := r.readBlockList(runID, artifact.ID) - chunks, err = listOrderedChunksByRunID(r.fs, runID, artifact.ID, blockList) + chunks, err = listOrderedChunksForArtifact(r.fs, runID, artifact.ID, blockList) if err != nil { log.Error("Error list chunks: %v", errors.Join(blockListErr, err)) ctx.HTTPError(http.StatusInternalServerError, "Error list chunks") From 46977d51ba78d062fe20fef5944f4b1a6142a6c8 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 4 Mar 2026 07:25:09 +0800 Subject: [PATCH 21/26] refactor tmp path name (TODO) --- routers/api/actions/artifacts_chunks.go | 16 +++++++++++++--- routers/api/actions/artifactsv4.go | 6 +++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index 56243257dbeb7..a11d912630b45 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -30,6 +30,16 @@ type saveUploadChunkOptions struct { checkMd5 bool } +func makeTmpPathNameV3(runID int64) string { + // TODO: maybe use "tmp-upload/run-12345" in the future + return fmt.Sprintf("tmp%d", runID) +} + +func makeTmpPathNameV4(runID int64) string { + // TODO: maybe use "tmp-upload/run-12345-v4" in the future + return fmt.Sprintf("tmpv4%d", runID) +} + func makeChunkFilenameV3(runID, artifactID, start int64, endPtr *int64) string { var end int64 if endPtr != nil { @@ -68,7 +78,7 @@ func saveUploadChunkV3(st storage.ObjectStorage, ctx *ArtifactContext, artifact runID int64, opts saveUploadChunkOptions, ) (writtenSize int64, retErr error) { // build chunk store path - storagePath := fmt.Sprintf("tmp%d/%s", runID, makeChunkFilenameV3(runID, artifact.ID, opts.start, opts.end)) + storagePath := fmt.Sprintf("%s/%s", makeTmpPathNameV3(runID), makeChunkFilenameV3(runID, artifact.ID, opts.start, opts.end)) // "end" is optional, so "contentSize=-1" means read until EOF contentSize := int64(-1) @@ -152,7 +162,7 @@ type chunkFileItem struct { } func listV3UnorderedChunksMapByRunID(st storage.ObjectStorage, runID int64) (map[int64][]*chunkFileItem, error) { - storageDir := fmt.Sprintf("tmp%d", runID) + storageDir := makeTmpPathNameV3(runID) var chunks []*chunkFileItem if err := st.IterateObjects(storageDir, func(fpath string, obj storage.Object) error { item, err := parseChunkFileItemV3(st, fpath) @@ -182,7 +192,7 @@ func listOrderedChunksForArtifact(st storage.ObjectStorage, runID, artifactID in return chunks, nil } - storageDir := fmt.Sprintf("tmpv4%d", runID) + storageDir := makeTmpPathNameV4(runID) var chunks []*chunkFileItem var chunkMapV4 map[string]*chunkFileItem diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 108d5a6bb0d03..d208785f638d9 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -377,7 +377,7 @@ func (r *artifactV4Routes) uploadArtifact(ctx *ArtifactContext) { } } else { blockFilename := makeBlockFilenameV4(task.Job.RunID, artifact.ID, ctx.Req.ContentLength, blockID) - _, err := r.fs.Save(fmt.Sprintf("tmpv4%d/%s", task.Job.RunID, blockFilename), ctx.Req.Body, ctx.Req.ContentLength) + _, err := r.fs.Save(fmt.Sprintf("%s/%s", makeTmpPathNameV4(task.Job.RunID), blockFilename), ctx.Req.Body, ctx.Req.ContentLength) if err != nil { log.Error("Error uploading block blob %v", err) ctx.HTTPError(http.StatusInternalServerError, "Error uploading block blob") @@ -388,7 +388,7 @@ func (r *artifactV4Routes) uploadArtifact(ctx *ArtifactContext) { case "blocklist": rawArtifactID := ctx.Req.URL.Query().Get("artifactID") artifactID, _ := strconv.ParseInt(rawArtifactID, 10, 64) - _, err := r.fs.Save(fmt.Sprintf("tmpv4%d/%d-%d-blocklist", task.Job.RunID, task.Job.RunID, artifactID), ctx.Req.Body, -1) + _, err := r.fs.Save(fmt.Sprintf("%s/%d-%d-blocklist", makeTmpPathNameV4(task.Job.RunID), task.Job.RunID, artifactID), ctx.Req.Body, -1) if err != nil { log.Error("Error uploading blocklist %v", err) ctx.HTTPError(http.StatusInternalServerError, "Error uploading blocklist") @@ -407,7 +407,7 @@ type Latest struct { } func (r *artifactV4Routes) readBlockList(runID, artifactID int64) (*BlockList, error) { - blockListName := fmt.Sprintf("tmpv4%d/%d-%d-blocklist", runID, runID, artifactID) + blockListName := fmt.Sprintf("%s/%d-%d-blocklist", makeTmpPathNameV4(runID), runID, artifactID) s, err := r.fs.Open(blockListName) if err != nil { return nil, err From c2948039c09c8d3224f0f4df11040871c2734a50 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 4 Mar 2026 21:14:50 +0100 Subject: [PATCH 22/26] refactor tmp path name --- routers/api/actions/artifacts_chunks.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index a11d912630b45..a94c100983cfa 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -31,13 +31,11 @@ type saveUploadChunkOptions struct { } func makeTmpPathNameV3(runID int64) string { - // TODO: maybe use "tmp-upload/run-12345" in the future - return fmt.Sprintf("tmp%d", runID) + return fmt.Sprintf("tmp-upload/run-%d", runID) } func makeTmpPathNameV4(runID int64) string { - // TODO: maybe use "tmp-upload/run-12345-v4" in the future - return fmt.Sprintf("tmpv4%d", runID) + return fmt.Sprintf("tmp-upload/run-%d-v4", runID) } func makeChunkFilenameV3(runID, artifactID, start int64, endPtr *int64) string { From 56358a4782416eef6711b9cdce4e8494194770fa Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 5 Mar 2026 08:56:57 +0800 Subject: [PATCH 23/26] delete empty parent dirs --- modules/storage/local.go | 45 ++++++++++++++++++++++++---------- modules/storage/local_test.go | 46 +++++++++++++++++++++++++++++++++++ modules/util/path.go | 7 +++--- 3 files changed, 82 insertions(+), 16 deletions(-) diff --git a/modules/storage/local.go b/modules/storage/local.go index b534378be1ccb..5ea6f055ce750 100644 --- a/modules/storage/local.go +++ b/modules/storage/local.go @@ -28,25 +28,32 @@ type LocalStorage struct { // NewLocalStorage returns a local files func NewLocalStorage(ctx context.Context, config *setting.Storage) (ObjectStorage, error) { + // prepare storage root path if !filepath.IsAbs(config.Path) { - return nil, fmt.Errorf("LocalStorageConfig.Path should have been prepared by setting/storage.go and should be an absolute path, but not: %q", config.Path) - } - log.Info("Creating new Local Storage at %s", config.Path) - if err := os.MkdirAll(config.Path, os.ModePerm); err != nil { - return nil, err + return nil, fmt.Errorf("LocalStorage config.Path should have been prepared by setting/storage.go and should be an absolute path, but not: %q", config.Path) } + storageRoot := util.FilePathJoinAbs(config.Path) - if config.TemporaryPath == "" { - config.TemporaryPath = filepath.Join(config.Path, "tmp") + // prepare storage temporary path + storageTmp := config.TemporaryPath + if storageTmp == "" { + storageTmp = filepath.Join(storageRoot, "tmp") + } + if !filepath.IsAbs(storageTmp) { + return nil, fmt.Errorf("LocalStorage config.TemporaryPath should be an absolute path, but not: %q", config.TemporaryPath) } - if !filepath.IsAbs(config.TemporaryPath) { - return nil, fmt.Errorf("LocalStorageConfig.TemporaryPath should be an absolute path, but not: %q", config.TemporaryPath) + storageTmp = util.FilePathJoinAbs(storageTmp) + + // create the storage root if not exist + log.Info("Creating new Local Storage at %s", storageRoot) + if err := os.MkdirAll(storageRoot, os.ModePerm); err != nil { + return nil, err } return &LocalStorage{ ctx: ctx, - dir: config.Path, - tmpdir: config.TemporaryPath, + dir: storageRoot, + tmpdir: storageTmp, }, nil } @@ -109,9 +116,21 @@ func (l *LocalStorage) Stat(path string) (os.FileInfo, error) { return os.Stat(l.buildLocalPath(path)) } -// Delete delete a file +func (l *LocalStorage) deleteEmptyParentDirs(localFullPath string) { + for parent := filepath.Dir(localFullPath); len(parent) > len(l.dir); parent = filepath.Dir(parent) { + if err := os.Remove(parent); err != nil { + // since the target file has been deleted, parent dir error is not related to the file deletion itself. + break + } + } +} + +// Delete deletes the file in storage and removes the empty parent directories (if possible) func (l *LocalStorage) Delete(path string) error { - return util.Remove(l.buildLocalPath(path)) + localFullPath := l.buildLocalPath(path) + err := util.Remove(localFullPath) + l.deleteEmptyParentDirs(localFullPath) + return err } // URL gets the redirect URL to a file diff --git a/modules/storage/local_test.go b/modules/storage/local_test.go index 0592fd716ba16..29c13539d0e47 100644 --- a/modules/storage/local_test.go +++ b/modules/storage/local_test.go @@ -4,11 +4,14 @@ package storage import ( + "os" + "strings" "testing" "code.gitea.io/gitea/modules/setting" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestBuildLocalPath(t *testing.T) { @@ -53,6 +56,49 @@ func TestBuildLocalPath(t *testing.T) { } } +func TestLocalStorageDelete(t *testing.T) { + rootDir := t.TempDir() + st, err := NewLocalStorage(t.Context(), &setting.Storage{Path: rootDir}) + require.NoError(t, err) + + assertExists := func(t *testing.T, path string, exists bool) { + _, err = os.Stat(rootDir + "/" + path) + if exists { + require.NoError(t, err) + } else { + require.ErrorIs(t, err, os.ErrNotExist) + } + } + + _, err = st.Save("dir/sub1/1-a.txt", strings.NewReader(""), -1) + require.NoError(t, err) + _, err = st.Save("dir/sub1/1-b.txt", strings.NewReader(""), -1) + require.NoError(t, err) + _, err = st.Save("dir/sub2/2-a.txt", strings.NewReader(""), -1) + require.NoError(t, err) + + assertExists(t, "dir/sub1/1-a.txt", true) + assertExists(t, "dir/sub1/1-b.txt", true) + assertExists(t, "dir/sub2/2-a.txt", true) + + require.NoError(t, st.Delete("dir/sub1/1-a.txt")) + assertExists(t, "dir/sub1", true) + assertExists(t, "dir/sub1/1-a.txt", false) + assertExists(t, "dir/sub1/1-b.txt", true) + assertExists(t, "dir/sub2/2-a.txt", true) + + require.NoError(t, st.Delete("dir/sub1/1-b.txt")) + assertExists(t, ".", true) + assertExists(t, "dir/sub1", false) + assertExists(t, "dir/sub1/1-a.txt", false) + assertExists(t, "dir/sub1/1-b.txt", false) + assertExists(t, "dir/sub2/2-a.txt", true) + + require.NoError(t, st.Delete("dir/sub2/2-a.txt")) + assertExists(t, ".", true) + assertExists(t, "dir", false) +} + func TestLocalStorageIterator(t *testing.T) { testStorageIterator(t, setting.LocalStorageType, &setting.Storage{Path: t.TempDir()}) } diff --git a/modules/util/path.go b/modules/util/path.go index 48447d7b90525..508cb345550dc 100644 --- a/modules/util/path.go +++ b/modules/util/path.go @@ -78,11 +78,12 @@ func FilePathJoinAbs(base string, sub ...string) string { elems := make([]string, 1, len(sub)+1) // POSIX filesystem can have `\` in file names. Windows: `\` and `/` are both used for path separators - // to keep the behavior consistent, we do not allow `\` in file names, replace all `\` with `/` + // to keep the behavior consistent, we do not allow `\` in file names, replace all `\` with `/`. + // The base path will be cleaned together at the last step (return) by "Join". if isOSWindows() { - elems[0] = filepath.Clean(base) + elems[0] = base } else { - elems[0] = filepath.Clean(strings.ReplaceAll(base, "\\", filepathSeparator)) + elems[0] = strings.ReplaceAll(base, "\\", filepathSeparator) } if !filepath.IsAbs(elems[0]) { // This shouldn't happen. If there is really necessary to pass in relative path, return the full path with filepath.Abs() instead From 29dc6eef87c716424e59360cec7b225e4e93ddd5 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 5 Mar 2026 09:11:22 +0800 Subject: [PATCH 24/26] fine tune FilePathJoinAbs --- modules/util/path.go | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/modules/util/path.go b/modules/util/path.go index 508cb345550dc..baed03b443c2c 100644 --- a/modules/util/path.go +++ b/modules/util/path.go @@ -75,20 +75,21 @@ const filepathSeparator = string(os.PathSeparator) // {`/foo`, ``, `bar`} => `/foo/bar` // {`/foo`, `..`, `bar`} => `/foo/bar` func FilePathJoinAbs(base string, sub ...string) string { - elems := make([]string, 1, len(sub)+1) - // POSIX filesystem can have `\` in file names. Windows: `\` and `/` are both used for path separators - // to keep the behavior consistent, we do not allow `\` in file names, replace all `\` with `/`. - // The base path will be cleaned together at the last step (return) by "Join". - if isOSWindows() { - elems[0] = base - } else { - elems[0] = strings.ReplaceAll(base, "\\", filepathSeparator) + // to keep the behavior consistent, we do not allow `\` in file names, replace all `\` with `/` + if !isOSWindows() { + base = strings.ReplaceAll(base, "\\", filepathSeparator) + } + if !filepath.IsAbs(base) { + // This shouldn't happen. If it is really necessary to handle relative paths, use filepath.Abs() to get absolute paths first + panic(fmt.Sprintf("FilePathJoinAbs: %q (for path %v) is not absolute, do not guess a relative path based on current working directory", base, sub)) } - if !filepath.IsAbs(elems[0]) { - // This shouldn't happen. If there is really necessary to pass in relative path, return the full path with filepath.Abs() instead - panic(fmt.Sprintf("FilePathJoinAbs: %q (for path %v) is not absolute, do not guess a relative path based on current working directory", elems[0], elems)) + if len(sub) == 0 { + return filepath.Clean(base) } + + elems := make([]string, 1, len(sub)+1) + elems[0] = base for _, s := range sub { if s == "" { continue @@ -99,7 +100,7 @@ func FilePathJoinAbs(base string, sub ...string) string { elems = append(elems, filepath.Clean(filepathSeparator+strings.ReplaceAll(s, "\\", filepathSeparator))) } } - // the elems[0] must be an absolute path, just join them together + // the elems[0] must be an absolute path, just join them together, and Join will also do Clean return filepath.Join(elems...) } From 349bc11a09f41457681f49c36965ee5968a6fb4e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 5 Mar 2026 09:31:18 +0800 Subject: [PATCH 25/26] fix --- modules/storage/storage.go | 2 +- routers/api/actions/artifacts_chunks.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/modules/storage/storage.go b/modules/storage/storage.go index 3ead05a669539..74d0cd47c874b 100644 --- a/modules/storage/storage.go +++ b/modules/storage/storage.go @@ -70,7 +70,7 @@ type ObjectStorage interface { URL(path, name, method string, reqParams url.Values) (*url.URL, error) // IterateObjects calls the iterator function for each object in the storage with the given path as prefix - // The "fullPath" in is the full path in this storage. + // The "fullPath" argument in callback is the full path in this storage. // * IterateObjects("", ...): iterate all objects in this storage // * IterateObjects("sub-path", ...): iterate all objects with "sub-path" as prefix in this storage, the "fullPath" will be like "sub-path/xxx" IterateObjects(basePath string, iterator func(fullPath string, obj Object) error) error diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index a94c100983cfa..13ed90385b829 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -195,6 +195,7 @@ func listOrderedChunksForArtifact(st storage.ObjectStorage, runID, artifactID in var chunkMapV4 map[string]*chunkFileItem if blist != nil { + // make a dummy map for quick lookup of chunk names, the values are nil now and will be filled after iterating storage objects chunkMapV4 = map[string]*chunkFileItem{} for _, name := range blist.Latest { chunkMapV4[name] = nil @@ -237,8 +238,8 @@ func listOrderedChunksForArtifact(st storage.ObjectStorage, runID, artifactID in if len(chunks) == 0 && blist != nil { for i, name := range blist.Latest { - chunk, ok := chunkMapV4[name] - if !ok || chunk.Path == "" { + chunk := chunkMapV4[name] + if chunk == nil { return nil, fmt.Errorf("missing chunk (%d/%d): %s", i, len(blist.Latest), name) } chunks = append(chunks, chunk) From a569179dd6b55370e9f3c7a5e0490a6025eb7ad5 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 5 Mar 2026 22:53:09 +0800 Subject: [PATCH 26/26] use pointer instead of new() Signed-off-by: wxiaoguang --- routers/api/actions/artifacts_chunks.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index 13ed90385b829..86a51d6ca64bc 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -131,7 +131,7 @@ func saveUploadChunkV3GetTotalSize(st storage.ObjectStorage, ctx *ArtifactContex if _, err := fmt.Sscanf(contentRange, "bytes %d-%d/%d", &start, &end, &totalSize); err != nil { return 0, fmt.Errorf("parse content range error: %v", err) } - _, err := saveUploadChunkV3(st, ctx, artifact, runID, saveUploadChunkOptions{start: start, end: new(end), checkMd5: true}) + _, err := saveUploadChunkV3(st, ctx, artifact, runID, saveUploadChunkOptions{start: start, end: &end, checkMd5: true}) if err != nil { return 0, err } @@ -142,7 +142,8 @@ func saveUploadChunkV3GetTotalSize(st storage.ObjectStorage, ctx *ArtifactContex func appendUploadChunkV3(st storage.ObjectStorage, ctx *ArtifactContext, artifact *actions.ActionArtifact, runID, start int64) (int64, error) { opts := saveUploadChunkOptions{start: start} if ctx.Req.ContentLength > 0 { - opts.end = new(start + ctx.Req.ContentLength - 1) + end := start + ctx.Req.ContentLength - 1 + opts.end = &end } return saveUploadChunkV3(st, ctx, artifact, runID, opts) }