Repository navigation
Conversation
Coverage Report for CI Build 36944784621Coverage increased (+0.08%) to 83.912%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats💛 - Coveralls |
This branch has not been deployed
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.
Summary
Combine valid user-metadata entries whose names differ only in case when returning S3 HEAD/GET responses. Add ten AWS SDK-deserialized regression cases covering both commands.
Problem and root cause
Fixes #1461.
Storage JSON metadata can contain both
Titleandtitle. The sharedtoAwsMetadataHeadersmapper lowercases names but overwrites earlier entries, so S3 clients silently receive only the last value. S3's documented behavior combines same-name headers, ignoring case, into a comma-delimited list.Solution
Append colliding valid values with a comma and validate the resulting header before assigning it. Preserve the existing ASCII/name/value validation, header-length limit, and
x-amz-missing-metaaccounting. HEAD and GET already share this mapper; no route or upload changes are needed.Tests
The first regression failed on clean current master
76b70ba8: expectedfirst,second, receivedsecond. It passed after the mapper change. Tests use the real SDK deserializer and production mapper with in-memory storage boundaries; no network or live credentials are required.Coverage: two/three colliding names, empty values, existing commas, distinct names, invalid entries, and combined-header overflow, for both HEAD and GET.
npm run test:unit -- src/storage/protocols/s3/s3-metadata.test.ts src/storage/protocols/s3/s3-handler.test.ts src/http/routes/s3/index.test.ts src/http/routes/s3/router.test.ts src/internal/http/header.test.ts --maxWorkers=2 --testTimeout=30000 --hookTimeout=30000 # 247 passed across 5 filesValidation
npm run test:unit -- --maxWorkers=2 --testTimeout=30000 --hookTimeout=30000: 2,551 passed across 160 files.npm run build: passed, including TypeScript checking.npm run lint: passed, including repository-wide Biome and Prettier.git diff --check: passed.Compatibility, documentation, and prior work
Only colliding response metadata changes; ordinary headers and invalid-entry handling remain unchanged. No API/schema/dependency changes or documentation updates are needed for this S3-compatibility fix. Searched issues, PRs, discussions and history for the collision behavior. Active #1455 validates decoded metadata shape and does not modify this response mapper.