Skip to content

fix(storage): bounds-check the file basename in PurgeUploads Walk callback#4860

Merged
milosgajdos merged 1 commit into
distribution:mainfrom
SAY-5:fix/purge-uploads-empty-file-basename-4713
Apr 22, 2026
Merged

fix(storage): bounds-check the file basename in PurgeUploads Walk callback#4860
milosgajdos merged 1 commit into
distribution:mainfrom
SAY-5:fix/purge-uploads-empty-file-basename-4713

Conversation

@SAY-5

@SAY-5 SAY-5 commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

What

Fixes #4713.

PurgeUploads' Walk callback split the visited path with path.Split and then indexed file[0] immediately. path.Split returns an empty basename for paths that end in a trailing slash - in practice this happens when an S3 driver surfaces a bare directory (common prefix) with an empty Key. Indexing a zero-length string panics with index out of range [0] with length 0 and takes down the whole PurgeUploads goroutine, as seen in the report's trace (purgeuploads.go:73s3.go:1023).

Fix

Guard the length before touching file[0] so a trailing-slash / empty-basename entry is skipped as 'not a reserved directory' - matching what the branch was trying to do anyway. Runtime behaviour for every non-empty entry is unchanged; only the zero-length edge case stops crashing.

Verification

Locally on macOS, go 1.26.2:

  • gofmt -s -l registry/storage/purgeuploads.go: clean
  • go vet ./registry/storage/...: clean
  • go test -count=1 -run TestPurge ./registry/storage/: pass

Closes #4713

…lback

PurgeUploads' Walk callback split the visited path with path.Split and
indexed file[0] immediately. path.Split returns an empty basename for
paths that end in a trailing slash - in practice this happens when an
S3 driver surfaces a bare directory (common prefix) with an empty
Key. Indexing a zero-length string then panics with
'index out of range [0] with length 0' and takes down the whole
PurgeUploads goroutine (distribution#4713).

Guard the length before touching file[0] so a trailing-slash /
empty-basename entry is simply skipped as 'not a reserved directory',
which matches what the branch was trying to do anyway. Runtime
behaviour for every non-empty entry is unchanged.

Closes distribution#4713

Signed-off-by: SAY-5 <SAY-5@users.noreply.github.com>
@milosgajdos milosgajdos requested review from joaodrp and squizzi April 21, 2026 21:51

@squizzi squizzi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for including a code comment describing the fix.

@milosgajdos milosgajdos merged commit f3af4de into distribution:main Apr 22, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PurgeUploads panic

3 participants