Skip to content

HDDS-12911. Add key name validation to AWS S3 PutObject API #8453

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sreejasahithi
Copy link
Contributor

@sreejasahithi sreejasahithi commented May 14, 2025

What changes were proposed in this pull request?

AWS S3 PutObject API allows creating keys and directories with names containing invalid characters, but delete operation fails to remove such invalid objects from a FSO bucket. Hence to resolve this issue, key name validation is added to reject invalid key and directory names at creation time.

For example :
Before key name validation:
aws s3api --endpoint http://localhost:9878/ put-object --bucket fsobuck --key sampledir:1/ --no-verify-ssl
this has no error and gets created successfully.
but when we try to delete it we get an error as follows,
An error occurred (500) when calling the DeleteObject operation (reached max retries: 4): Internal Server Error

After key name validation:
aws s3api --endpoint http://localhost:9878/ put-object --bucket fsobuck --key sampledir:1/ --no-verify-ssl
this would return
An error occurred (InvalidRequest) when calling the PutObject operation: Invalid key path: key names cannot include: \\ { } < > ^ % ~ # | [ ] : "  or any non-ASCII characters.`
Hence the object who's name contains invalid characters is not created.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-12911

How was this patch tested?

https://github.com/sreejasahithi/ozone/actions/runs/15020991860

This change was also tested locally over a docker ozone cluster.

@sreejasahithi sreejasahithi marked this pull request as ready for review May 14, 2025 14:03
Copy link
Contributor

@sarvekshayr sarvekshayr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this, @sreejasahithi.
I tested the validation, and it works as expected.

Currently, characters like ,, ., .., etc., are allowed during the put-object operation, and delete-object is able to handle such entries unlike the : case. We can leave these as is for now and focus only on excluding : as implemented.

cc: @nandakumar131

String[] components = path.split("/");

for (String component : components) {
if (component.contains(":")) {
Copy link
Contributor

@sadanand48 sadanand48 May 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of just checking for a ":", we should do so a regex check like what's done in HDDS-5161.
Also add a unit/robot test to show that an invalid key name gets created before your fix is not being allowed now.

@adoroszlai adoroszlai added the s3 S3 Gateway label May 15, 2025
@sreejasahithi sreejasahithi requested a review from sadanand48 May 15, 2025 10:08
…ad of splitting and made error message robust
@sreejasahithi sreejasahithi requested a review from sadanand48 May 19, 2025 05:36
@sadanand48
Copy link
Contributor

sadanand48 commented May 20, 2025

I looked into this and the reason for the corner case of put-object allowing a key like "sample:/" is because if the --body is not provided , it gets translated into a CreateDirectory operation for an FSO bucket

$ aws s3api --endpoint http://xx:9878 put-object --bucket buck2 --key sample:/

$ aws s3api --endpoint http://xx:9878 delete-object --bucket buck2 --key sample:/

An error occurred (500) when calling the DeleteObject operation (reached max retries: 2): Internal Server Error

The keyName check is controlled by a flag ozone.om.keyname.character.check.enabled which was introduced here and defaults to false.
But the logic in OmDeleteKeyRequest is different i.e it does not call the same validateKey() method but instead calls validateAndNormalizeKey which internally only validates if the bucket is FSO.

To not allow these chars in the first place, I feel one should tune this config to true for stricter char check or we could even change default to true.

@adoroszlai what do you think?.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s3 S3 Gateway
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants