Skip to content

fix: add bucket name length constraint #685

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

Merged
merged 6 commits into from
May 12, 2025

Conversation

TylerHillery
Copy link
Contributor

@TylerHillery TylerHillery commented May 3, 2025

What kind of change does this PR introduce?

Bug fix, restricting bucket name length to a max of 63 characters to adhere to S3 Bucket naming rules

What is the current behavior?

No restrictions on the length of bucket name

What is the new behavior?

Added Postgres buckets name length check constraint and updated isValidBucketName function. Also added test in bucket.test.ts to verify 400 is being returned when issuing a POST request with name longer than 63 characters.

Additional context

I updated the fromDBerror switch statement to include the new pgError.code for the buckets name length check constraint. The ERRORS.InvalidBucketName error requires the bucket name I opted to parse it from the pgError.detail Example pgError.detail:

'Failing row contains (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, null, 2025-05-03 21:04:15.907656+00, 2025-05-03 21:04:15.907656+00, f, f, null, null, null).'

@TylerHillery TylerHillery marked this pull request as ready for review May 5, 2025 14:34
@TylerHillery TylerHillery requested review from fenos and itslenny May 5, 2025 14:34
@TylerHillery TylerHillery requested a review from itslenny May 7, 2025 01:31
@TylerHillery
Copy link
Contributor Author

Updated PR to change the max bucket name length to 100 to stay backwards compatible

@TylerHillery TylerHillery marked this pull request as draft May 8, 2025 16:42
@TylerHillery
Copy link
Contributor Author

We can't add postgres constraint if there is data already in the table that violates the constraint. Putting this PR back in draft while I think of a way we can implement this while maintaining compatibility with buckets that already violate the constraint.

@TylerHillery TylerHillery marked this pull request as ready for review May 12, 2025 14:00
@TylerHillery TylerHillery merged commit a33d97d into master May 12, 2025
1 of 2 checks passed
@TylerHillery TylerHillery deleted the tyler/fix/restrict-bucket-name-length branch May 12, 2025 15:19
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.

3 participants