fix(api): avoid pull files panic with whitespace ignored - #39196
Open
lunny wants to merge 4 commits into
Open
Conversation
The pull request files endpoint used the shortstat file count to page through the filtered diff. When whitespace changes are ignored, the shortstat can report more files than the diff contains, causing an index out of range panic. Apply the whitespace behavior to the shortstat and clamp the page to the files returned by the diff. Assisted-by: Codet:unknown
GiteaBot
added
the
lgtm/need 2
Reuse the same DiffOptions for both the file diff and the shortstat so options such as whitespace, rename detection, and skip-to stay consistent between the two commands. Assisted-by: Codet:unknown
Use the same DiffOptions for the rendered diff and its shortstat on pull, commit, and compare pages. This keeps whitespace, skip-to, and file filters from making the summary count diverge from the visible diff. Assisted-by: Codet:unknown
The shortstat and diff now share the same DiffOptions, so the extra clamp against len(diff.Files) is no longer needed. Assisted-by: Codet:unknown
Member
|
I wonder if this can be fixed without introducing another |
Member
Author
One is for the stats, another is for the pagination(max files). So that I don't think it can be done in one git call. |
wxiaoguang
reviewed
Sep 5, 2026
| return GetDiffShortStatByCmd(ctx, repo, cmd) | ||
| } | ||
|
|
||
| func GetDiffShortStatByCmd(ctx context.Context, repo RepositoryFacade, cmd *gitcmd.Command) (numFiles, totalAdditions, totalDeletions int, err error) { |
Contributor
There was a problem hiding this comment.
You should really read the code and think about what's the right thing to do
// TODO: it can be merged with another "GetDiffShortStat" in the future
Contributor
There was a problem hiding this comment.
refactor: GetDiffShortStat and fix panic caused by inconsistent "changed file number" - #39248
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The pull request files endpoint paginated by the shortstat file count while indexing into the filtered diff. With
whitespace=ignore-change, git omits whitespace-only files, so the shortstat can report more files than the diff contains and the handler panics with an index out of range error.Build both the diff and the shortstat from the same
DiffOptionsso whitespace, rename detection, and skip-to stay consistent, and clamp the page to the files actually returned by the diff.Fixes #39191