feat(redirection): support directory-form internal targets#1098
Merged
Conversation
A CSV row whose target is an internal directory path (e.g. /503/) was rejected by the upload validation, because HeadObject was called with the literal key "503/" which never exists on S3. Resolve directory-form targets to their index document (index.html) before checking existence, matching how the S3 website endpoint serves such URLs. Also keep the trailing slash when writing WebsiteRedirectLocation: path.Join collapses "/503/" to "/503", which costs visitors an extra redirect hop (the website endpoint 302s /503 back to /503/) on every hit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Go | Jul 17, 2026 7:32a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
zhangshanwen
approved these changes
Jul 17, 2026
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
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
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.
Problem
Uploading a redirection CSV with an internal directory-form target fails validation:
The upload is rejected with Target object not existed, because the existence check calls
HeadObjectwith the literal key503/, which never exists on S3 — only503/index.htmldoes. Users are forced to write/503/index.htmleven though/503/is a perfectly valid URL on the S3 website endpoint (served via the index document rule).Changes
checkTargetExists, new): a target ending in/now resolves to its index document (index.html) before theHeadObjectexistence check, matching how the S3 website endpoint serves such URLs. This also makes/(site root) usable as a target. ThecheckObjectExistshelper is unchanged, so the source-side existence check inredirection()keeps its current behavior.redirection()): keep the trailing slash when writingWebsiteRedirectLocation. Previouslypath.Joincollapsed/503/to/503, which costs visitors an extra redirect hop on every hit (the website endpoint 302s/503back to/503/before serving the index document).Tests
TestCheckTargetExists— directory form resolves to index document; root resolves to index document; missing objects still rejected; no-trailing-slash paths are not treated as directories (6 cases, against a mock S3 HTTP endpoint)TestRedirectionKeepsDirectoryTargetSlash— asserts thex-amz-website-redirect-locationactually sent to S3 keeps the trailing slash for directory-form targets and is unchanged for file / absolute-URL targets (3 cases)./redirectionsuite passes🤖 Generated with Claude Code