Skip to content

Commit dd661e9

Browse files
GiteaBotlunnywxiaoguang
authored
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 <[email protected]> Co-authored-by: wxiaoguang <[email protected]>
1 parent 0b31272 commit dd661e9

File tree

4 files changed

+28
-12
lines changed

4 files changed

+28
-12
lines changed

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@ This changelog goes through the changes that have been made in each release
44
without substantial changes to our git log; to see the highlights of what has
55
been added to each release, please refer to the [blog](https://blog.gitea.com).
66

7-
## [1.24.3](https://github.com/go-gitea/gitea/releases/tag/1.24.3) - 2025-07-11
7+
## [1.24.3](https://github.com/go-gitea/gitea/releases/tag/1.24.3) - 2025-07-12
88

99
* BUGFIXES
10+
* Fix incorrect comment diff hunk parsing, fix github asset ID nil panic (#35046) (#35055)
1011
* Fix updating user visibility (#35036) (#35044)
1112
* Support base64-encoded agit push options (#35037) (#35041)
1213
* Make submodule link work with relative path (#35034) (#35038)

modules/git/diff.go

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,9 @@ func GetRepoRawDiffForFile(repo *Repository, startCommit, endCommit string, diff
9999
return nil
100100
}
101101

102-
// ParseDiffHunkString parse the diffhunk content and return
103-
func ParseDiffHunkString(diffhunk string) (leftLine, leftHunk, rightLine, righHunk int) {
104-
ss := strings.Split(diffhunk, "@@")
102+
// ParseDiffHunkString parse the diff hunk content and return
103+
func ParseDiffHunkString(diffHunk string) (leftLine, leftHunk, rightLine, rightHunk int) {
104+
ss := strings.Split(diffHunk, "@@")
105105
ranges := strings.Split(ss[1][1:], " ")
106106
leftRange := strings.Split(ranges[0], ",")
107107
leftLine, _ = strconv.Atoi(leftRange[0][1:])
@@ -112,14 +112,19 @@ func ParseDiffHunkString(diffhunk string) (leftLine, leftHunk, rightLine, righHu
112112
rightRange := strings.Split(ranges[1], ",")
113113
rightLine, _ = strconv.Atoi(rightRange[0])
114114
if len(rightRange) > 1 {
115-
righHunk, _ = strconv.Atoi(rightRange[1])
115+
rightHunk, _ = strconv.Atoi(rightRange[1])
116116
}
117117
} else {
118-
log.Debug("Parse line number failed: %v", diffhunk)
118+
log.Debug("Parse line number failed: %v", diffHunk)
119119
rightLine = leftLine
120-
righHunk = leftHunk
120+
rightHunk = leftHunk
121121
}
122-
return leftLine, leftHunk, rightLine, righHunk
122+
if rightLine == 0 {
123+
// FIXME: GIT-DIFF-CUT-BUG search this tag to see details
124+
// this is only a hacky patch, the rightLine&rightHunk might still be incorrect in some cases.
125+
rightLine++
126+
}
127+
return leftLine, leftHunk, rightLine, rightHunk
123128
}
124129

125130
// Example: @@ -1,8 +1,9 @@ => [..., 1, 8, 1, 9]
@@ -270,6 +275,12 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi
270275
oldNumOfLines++
271276
}
272277
}
278+
279+
// "git diff" outputs "@@ -1 +1,3 @@" for "OLD" => "A\nB\nC"
280+
// 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 @@"
281+
// 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.
282+
// For example: "L1\nL2" => "A\nB", then the patch shows "L2" as line 1 on the left (deleted part)
283+
273284
// construct the new hunk header
274285
newHunk[headerLines] = fmt.Sprintf("@@ -%d,%d +%d,%d @@",
275286
oldBegin, oldNumOfLines, newBegin, newNumOfLines)

services/gitdiff/gitdiff.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ func (d *DiffLine) GetExpandDirection() DiffLineExpandDirection {
179179
}
180180

181181
func getDiffLineSectionInfo(treePath, line string, lastLeftIdx, lastRightIdx int) *DiffLineSectionInfo {
182-
leftLine, leftHunk, rightLine, righHunk := git.ParseDiffHunkString(line)
182+
leftLine, leftHunk, rightLine, rightHunk := git.ParseDiffHunkString(line)
183183

184184
return &DiffLineSectionInfo{
185185
Path: treePath,
@@ -188,7 +188,7 @@ func getDiffLineSectionInfo(treePath, line string, lastLeftIdx, lastRightIdx int
188188
LeftIdx: leftLine,
189189
RightIdx: rightLine,
190190
LeftHunkSize: leftHunk,
191-
RightHunkSize: righHunk,
191+
RightHunkSize: rightHunk,
192192
}
193193
}
194194

@@ -335,7 +335,7 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc
335335
// try to find equivalent diff line. ignore, otherwise
336336
switch diffLine.Type {
337337
case DiffLineSection:
338-
return getLineContent(diffLine.Content[1:], locale)
338+
return getLineContent(diffLine.Content, locale)
339339
case DiffLineAdd:
340340
compareDiffLine := diffSection.GetLine(diffLine.Match)
341341
return diffSection.getDiffLineForRender(DiffLineAdd, compareDiffLine, diffLine, locale)
@@ -904,6 +904,7 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
904904
lastLeftIdx = -1
905905
curFile.Sections = append(curFile.Sections, curSection)
906906

907+
// FIXME: the "-1" can't be right, these "line idx" are all 1-based, maybe there are other bugs that covers this bug.
907908
lineSectionInfo := getDiffLineSectionInfo(curFile.Name, line, leftLine-1, rightLine-1)
908909
diffLine := &DiffLine{
909910
Type: DiffLineSection,

services/migrations/github.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,10 @@ func (g *GithubDownloaderV3) convertGithubRelease(ctx context.Context, rel *gith
322322
httpClient := NewMigrationHTTPClient()
323323

324324
for _, asset := range rel.Assets {
325-
assetID := *asset.ID // Don't optimize this, for closure we need a local variable
325+
assetID := asset.GetID() // Don't optimize this, for closure we need a local variable TODO: no need to do so in new Golang
326+
if assetID == 0 {
327+
continue
328+
}
326329
r.Assets = append(r.Assets, &base.ReleaseAsset{
327330
ID: asset.GetID(),
328331
Name: asset.GetName(),

0 commit comments

Comments
 (0)