-
Notifications
You must be signed in to change notification settings - Fork 188
Let S3Mock accept * and "*" for conditional requests #2382
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
Conversation
S3 API accepts * for conditional requests on all APIs. All APIs but PutObject succeed with "*" as well. Fixes #2371
68b8718
to
384ddda
Compare
230389e
to
9f506ba
Compare
9f506ba
to
9a1e155
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request enhances S3Mock’s conditional request handling by allowing both "" and """" as valid wildcard etags. In addition, tests have been refactored to use centralized file constants and bucket naming has been updated for consistency.
- Updates conditional matching logic in ObjectService to consider both wildcards.
- Replaces local File instantiation in tests with UPLOAD_FILE constants and improves resource usage.
- Refines bucket name generation in S3TestBase to replace "=" with "-" for consistency.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
server/src/main/java/com/adobe/testing/s3mock/service/ObjectService.java | Adjusted conditional etag matching and reordered date-based checks to support "*" wildcard requests. |
integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/* | Updated test files to use centralized UPLOAD_FILE constants and ensure proper resource management via .use. |
integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/S3TestBase.kt | Modified bucket naming logic by replacing "=" with "-" and added a helper method for directory buckets. |
Comments suppressed due to low confidence (2)
server/src/main/java/com/adobe/testing/s3mock/service/ObjectService.java:374
- [nitpick] Verify that moving the unmodified-since check to after the match condition does not introduce any unintended side effects in conditional request evaluation.
var setUnmodifiedSince = ifUnmodifiedSince != null && !ifUnmodifiedSince.isEmpty();
integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/S3TestBase.kt:248
- [nitpick] Confirm that replacing '=' with '-' in bucket names aligns with the expected S3 naming conventions and does not impact any external integrations.
.replace('=', '-')
if (setUnmodifiedSince && ifUnmodifiedSince.get(0).isBefore(lastModified)) { | ||
throw PRECONDITION_FAILED; | ||
} | ||
|
||
var setMatch = match != null && !match.isEmpty(); | ||
if (setMatch) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider adding an inline comment to explain why the condition now checks for both '' and '""', which will aid future maintainers in understanding the rationale behind expanding the wildcard support.
if (setMatch) { | |
if (setMatch) { | |
// Check for both quoted ("*") and unquoted (*) wildcards to ensure compatibility | |
// with different client implementations or specifications. |
Copilot uses AI. Check for mistakes.
f9e04c0
to
6e91c2d
Compare
Description
S3 API accepts * for conditional requests on all APIs. All APIs but PutObject succeed with "*" as well.
Related Issue
Fixes #2371
Tasks