fix: reject full-width space and tab in Bedrock document file names - #1677
Open
takke1986 wants to merge 2 commits into
Open
fix: reject full-width space and tab in Bedrock document file names#1677takke1986 wants to merge 2 commits into
takke1986 wants to merge 2 commits into
Conversation
convertToSafeFilename allowed \s through its character class, but in JavaScript \s also matches U+3000 (ideographic space) and tabs, which Bedrock's DocumentBlock.name rejects. Kanji and kana were replaced with _, so only whitespace characters slipped past the sanitizer and the attachment failed with ValidationException. - Allow only the ASCII space instead of \s - Collapse consecutive spaces, which Bedrock also rejects - Fall back to a placeholder when the sanitized name would be empty, since name has a minimum length of 1 - Apply the same fix to the AgentCore path in strandsUtils.ts Verified against bedrock-runtime:Converse in ap-northeast-1: the names this function used to emit are rejected and the new ones are accepted.
A reviewer pointed out on aws-samples#1671 that the length limit I left out of scope is documented after all. DocumentBlock.name carries "Minimum length of 1. Maximum length of 200." on the same API reference page that documents the whitespace rule this PR already fixes, so treating it as unspecified was my mistake. Both call sites now truncate. Where a hash suffix is added it is the hash that keeps two different names apart, so the base name is trimmed to make room rather than the suffix being dropped. Truncation can also leave a trailing space, which is not rejected but reads as an accident in the name the model is shown, so it is removed. Verified against bedrock-runtime:Converse in us-east-1: a 200-character name is accepted, a 201-character one is rejected with the message quoted on the issue.
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.
Description of Changes
convertToSafeFilenamesanitizes attachment file names for Bedrock'sDocumentBlock.name, but the character class allowed\s:In JavaScript
\salso matches U+3000 (ideographic space) and tabs, which Bedrock rejects. Kanji and kana were correctly replaced with_, so only whitespace characters slipped through the sanitizer. A file namedテスト 資料.xlsxtherefore failed with:Full-width spaces are common in Japanese file names, so this is hit frequently. The reporter of #1671 measured about 19% of attached documents failing for this reason.
This PR:
\snamehas a minimum length of 1)packages/web/src/utils/strandsUtils.ts, which had the identical\sin its character classNot in scope: the file name length limit mentioned in the issue. The exact limit is not documented, so I left it out rather than guess. Happy to add it as a follow-up if you would like.
Compatibility: names that were already accepted are unchanged. Only names containing non-ASCII whitespace or consecutive spaces produce a different result, and those previously failed the API call outright.
Verified against the Bedrock API
I called
bedrock-runtime:Converseinap-northeast-1withjp.anthropic.claude-haiku-4-5-20251001-v1:0, varying onlydocument.name, to confirm that the names this function used to emit are actually rejected and the new ones are accepted.document.nametest name(U+3000)test_name_706fc6f2test name(two ASCII spaces)test name_695fce01test\tname(tab)_______9c852928(all-Japanese name)file_d41d8cd9(empty-name fallback)test name(single ASCII space)This independently reproduces the table in #1671.
Checklist
npm run cdk:test— 45 tests / 15 snapshots passed, no snapshot differencesAlso run:
npm -w packages/cdk test -- fileNameUtils— 15 passed (9 existing + 6 added)npm run web:test— 278 passedeslintandprettier --checkclean on all changed filesRelated Issues