From dd661e92df2ac17010e47a5b86ff26c71859e63f Mon Sep 17 00:00:00 2001 From: Giteabot Date: Sat, 12 Jul 2025 15:12:02 +0800 Subject: [PATCH] Fix incorrect comment diff hunk parsing, fix github asset ID nil panic (#35046) (#35055) Backport #35046 by lunny * Fix missing the first char when parsing diff hunk header * Fix #35040 * Fix #35049 --------- Co-authored-by: Lunny Xiao Co-authored-by: wxiaoguang --- CHANGELOG.md | 3 ++- modules/git/diff.go | 25 ++++++++++++++++++------- services/gitdiff/gitdiff.go | 7 ++++--- services/migrations/github.go | 5 ++++- 4 files changed, 28 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b1d6d5e0c8..b363fc295d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,10 @@ This changelog goes through the changes that have been made in each release without substantial changes to our git log; to see the highlights of what has been added to each release, please refer to the [blog](https://blog.gitea.com). -## [1.24.3](https://github.com/go-gitea/gitea/releases/tag/1.24.3) - 2025-07-11 +## [1.24.3](https://github.com/go-gitea/gitea/releases/tag/1.24.3) - 2025-07-12 * BUGFIXES + * Fix incorrect comment diff hunk parsing, fix github asset ID nil panic (#35046) (#35055) * Fix updating user visibility (#35036) (#35044) * Support base64-encoded agit push options (#35037) (#35041) * Make submodule link work with relative path (#35034) (#35038) diff --git a/modules/git/diff.go b/modules/git/diff.go index c4df6b8063..35d115be0e 100644 --- a/modules/git/diff.go +++ b/modules/git/diff.go @@ -99,9 +99,9 @@ func GetRepoRawDiffForFile(repo *Repository, startCommit, endCommit string, diff return nil } -// ParseDiffHunkString parse the diffhunk content and return -func ParseDiffHunkString(diffhunk string) (leftLine, leftHunk, rightLine, righHunk int) { - ss := strings.Split(diffhunk, "@@") +// ParseDiffHunkString parse the diff hunk content and return +func ParseDiffHunkString(diffHunk string) (leftLine, leftHunk, rightLine, rightHunk int) { + ss := strings.Split(diffHunk, "@@") ranges := strings.Split(ss[1][1:], " ") leftRange := strings.Split(ranges[0], ",") leftLine, _ = strconv.Atoi(leftRange[0][1:]) @@ -112,14 +112,19 @@ func ParseDiffHunkString(diffhunk string) (leftLine, leftHunk, rightLine, righHu rightRange := strings.Split(ranges[1], ",") rightLine, _ = strconv.Atoi(rightRange[0]) if len(rightRange) > 1 { - righHunk, _ = strconv.Atoi(rightRange[1]) + rightHunk, _ = strconv.Atoi(rightRange[1]) } } else { - log.Debug("Parse line number failed: %v", diffhunk) + log.Debug("Parse line number failed: %v", diffHunk) rightLine = leftLine - righHunk = leftHunk + rightHunk = leftHunk } - return leftLine, leftHunk, rightLine, righHunk + if rightLine == 0 { + // FIXME: GIT-DIFF-CUT-BUG search this tag to see details + // this is only a hacky patch, the rightLine&rightHunk might still be incorrect in some cases. + rightLine++ + } + return leftLine, leftHunk, rightLine, rightHunk } // Example: @@ -1,8 +1,9 @@ => [..., 1, 8, 1, 9] @@ -270,6 +275,12 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi oldNumOfLines++ } } + + // "git diff" outputs "@@ -1 +1,3 @@" for "OLD" => "A\nB\nC" + // FIXME: GIT-DIFF-CUT-BUG But there is a bug in CutDiffAroundLine, then the "Patch" stored in the comment model becomes "@@ -1,1 +0,4 @@" + // It may generate incorrect results for difference cases, for example: delete 2 line add 1 line, delete 2 line add 2 line etc, need to double check. + // For example: "L1\nL2" => "A\nB", then the patch shows "L2" as line 1 on the left (deleted part) + // construct the new hunk header newHunk[headerLines] = fmt.Sprintf("@@ -%d,%d +%d,%d @@", oldBegin, oldNumOfLines, newBegin, newNumOfLines) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 32f9a82926..0e2819481e 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -179,7 +179,7 @@ func (d *DiffLine) GetExpandDirection() DiffLineExpandDirection { } func getDiffLineSectionInfo(treePath, line string, lastLeftIdx, lastRightIdx int) *DiffLineSectionInfo { - leftLine, leftHunk, rightLine, righHunk := git.ParseDiffHunkString(line) + leftLine, leftHunk, rightLine, rightHunk := git.ParseDiffHunkString(line) return &DiffLineSectionInfo{ Path: treePath, @@ -188,7 +188,7 @@ func getDiffLineSectionInfo(treePath, line string, lastLeftIdx, lastRightIdx int LeftIdx: leftLine, RightIdx: rightLine, LeftHunkSize: leftHunk, - RightHunkSize: righHunk, + RightHunkSize: rightHunk, } } @@ -335,7 +335,7 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc // try to find equivalent diff line. ignore, otherwise switch diffLine.Type { case DiffLineSection: - return getLineContent(diffLine.Content[1:], locale) + return getLineContent(diffLine.Content, locale) case DiffLineAdd: compareDiffLine := diffSection.GetLine(diffLine.Match) return diffSection.getDiffLineForRender(DiffLineAdd, compareDiffLine, diffLine, locale) @@ -904,6 +904,7 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact lastLeftIdx = -1 curFile.Sections = append(curFile.Sections, curSection) + // FIXME: the "-1" can't be right, these "line idx" are all 1-based, maybe there are other bugs that covers this bug. lineSectionInfo := getDiffLineSectionInfo(curFile.Name, line, leftLine-1, rightLine-1) diffLine := &DiffLine{ Type: DiffLineSection, diff --git a/services/migrations/github.go b/services/migrations/github.go index 662908756a..d0a0d71986 100644 --- a/services/migrations/github.go +++ b/services/migrations/github.go @@ -322,7 +322,10 @@ func (g *GithubDownloaderV3) convertGithubRelease(ctx context.Context, rel *gith httpClient := NewMigrationHTTPClient() for _, asset := range rel.Assets { - assetID := *asset.ID // Don't optimize this, for closure we need a local variable + assetID := asset.GetID() // Don't optimize this, for closure we need a local variable TODO: no need to do so in new Golang + if assetID == 0 { + continue + } r.Assets = append(r.Assets, &base.ReleaseAsset{ ID: asset.GetID(), Name: asset.GetName(),