Skip to content

feat(redirection): support directory-form internal targets#1098

Merged
Transmigration-zhou merged 1 commit into
mainfrom
feat/redirection-directory-target
Jul 17, 2026
Merged

feat(redirection): support directory-form internal targets#1098
Transmigration-zhou merged 1 commit into
mainfrom
feat/redirection-directory-target

Conversation

@Transmigration-zhou

Copy link
Copy Markdown
Contributor

Problem

Uploading a redirection CSV with an internal directory-form target fails validation:

source,target
/international/wen/test/index11.html,/503/

The upload is rejected with Target object not existed, because the existence check calls HeadObject with the literal key 503/, which never exists on S3 — only 503/index.html does. Users are forced to write /503/index.html even though /503/ is a perfectly valid URL on the S3 website endpoint (served via the index document rule).

Changes

  • Validation (checkTargetExists, new): a target ending in / now resolves to its index document (index.html) before the HeadObject existence check, matching how the S3 website endpoint serves such URLs. This also makes / (site root) usable as a target. The checkObjectExists helper is unchanged, so the source-side existence check in redirection() keeps its current behavior.
  • Write path (redirection()): keep the trailing slash when writing WebsiteRedirectLocation. Previously path.Join collapsed /503/ to /503, which costs visitors an extra redirect hop on every hit (the website endpoint 302s /503 back 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 the x-amz-website-redirect-location actually sent to S3 keeps the trailing slash for directory-form targets and is unchanged for file / absolute-URL targets (3 cases)
  • Full ./redirection suite passes

🤖 Generated with Claude Code

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>
@deepsource-io

deepsource-io Bot commented Jul 17, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in 99e1572...fd09e20 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

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.

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
redirection/event.go 0.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
redirection/storage.go 64.46% <100.00%> (+21.85%) ⬆️
redirection/event.go 74.71% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Transmigration-zhou
Transmigration-zhou merged commit 43d7321 into main Jul 17, 2026
11 of 15 checks passed
@Transmigration-zhou
Transmigration-zhou deleted the feat/redirection-directory-target branch July 17, 2026 07:39
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.

2 participants