Gargi-jais11 commented on PR #10860: URL: https://github.com/apache/ozone/pull/10860#issuecomment-5087752564
> I see that this logic was likely added based on the AWS S3 docs mentioning `'Non-printable ASCII characters (128–255 decimal characters)'` under the `'Characters to avoid'` section. > > However, there are two issues with enforcing it this way: AWS S3 doesn't actually block these: The AWS docs list this under `'Characters to avoid'` as a recommendation for clients to prevent application-side parsing issues. > > AWS S3 itself still accepts valid `UTF-8` characters that resolve to this range; it does not throw an InvalidURI error. > > This blocks valid international characters: Because I believe `keyPath` is a Java String (`UTF-16`), checking `c >= 0x80` && `c <= 0xFF` targets the `Unicode Latin-1 Supplement` block. This will hard-reject perfectly valid, printable characters like `é `(`U+00E9 / 233`), `ñ` (`U+00F1 / 241`), or `£` (`U+00A3 / 163`). > > A user uploading a file named `café.txt` or `piñata.jpg` will get an unexpected `InvalidURI` error. > > The Ceph `test_object_read_unreadable` test is likely sending raw, unencoded invalid bytes over the wire to trigger a parsing failure. We should rely on the `\uFFFD` check to catch malformed `UTF-8`, and if we want to block actual control characters, we should use `Character.isISOControl(c)` instead of blocking the entire `128-255` block. > > https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html#object-key-guidelines > > Or Am I missing somethign here? Thanks @ayushtkn for the review. You're right — the AWS "characters to avoid (128–255)" guidance is a recommendation, not something S3 enforces server-side, and blocking that range would incorrectly reject valid UTF-8 object names like café.txt. I didn't think about this, really a good catch. I've updated the validation to: - Reject keys containing \uFFFD (malformed UTF-8 after URI decoding) - Reject keys containing ISO control characters via Character.isISOControl() — this covers the s3-test case (U+008A in \xae\x8a-) - Removed the blanket 0x80–0xFF check and added a unit test confirming valid Unicode keys like café.txt are accepted. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
