Skip to content

Fix CopyObject panic when X-Amz-Copy-Source is fully URL-encoded#119

Open
rjpr wants to merge 1 commit into
johannesboyne:masterfrom
rjpr:fix-copy-source-url-decode
Open

Fix CopyObject panic when X-Amz-Copy-Source is fully URL-encoded#119
rjpr wants to merge 1 commit into
johannesboyne:masterfrom
rjpr:fix-copy-source-url-decode

Conversation

@rjpr

@rjpr rjpr commented Apr 16, 2026

Copy link
Copy Markdown

CopyObject panics with an index out of range when the X-Amz-Copy-Source header is fully URL-encoded:

panic: runtime error: index out of range [1] with length 1
    gofakes3.go:737 (*GoFakeS3).copyObject

The S3 API documentation requires the x-amz-copy-source header value to be URL-encoded, but is ambiguous about whether the "/" separating bucket and key should be encoded. Some clients encode only the key (bucket/src%2Bkey) while others encode the entire value, including the separator as "%2F" (bucket%2Fsrc%2Bkey).

gofakes3 only handles the first format, it splits on a literal "/" before URL-decoding, so a fully-encoded value produces a single-element slice, causing the panic.

This PR:

  1. Splits the source as-is first (preserves existing behavior)
  2. Falls back to URL-decoding and re-splitting if that produced <2 parts
  3. Skips the existing per-key url.QueryUnescape when the fallback path ran, to avoid double-decoding — QueryUnescape treats "+" as an encoded space, so decoding twice would corrupt any "+" in the key

Tests added:

  • Fully URL-encoded source path (the panic case)
  • Fully URL-encoded source with "+" and spaces in the key

Some S3 clients (notably AWS SDK v2 for Go) may URL-encode the entire
X-Amz-Copy-Source header value, including the "/" separator between
bucket and key as "%2F". This caused a panic when trying to access
parts[1] after splitting by "/", since the split produced only one
element.

This fix:
1. Attempts to split the source as-is (existing behavior)
2. If that fails (<2 parts), URL-decodes the source and tries again
3. Tracks whether decoding occurred to avoid double-decoding the key
   (which would corrupt "+" characters by converting them to spaces)

Added tests for:
- Fully URL-encoded source path (the panic case)
- Fully URL-encoded source with special chars like "+" and spaces

Fixes index out of range panic:
  panic: runtime error: index out of range [1] with length 1
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
75.3% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant