S3 MultipartUploads accepts case insensitive 'ETag' headers#11207
S3 MultipartUploads accepts case insensitive 'ETag' headers#11207jcborras wants to merge 4 commits intofluent:masterfrom
Conversation
WalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugins/out_s3/s3_multipart.c (1)
655-656: Case-insensitive ETag lookup looks correct; clean up commented code and confirmstrcasestrportabilitySwitching to
strcasestrhere correctly makes the ETag header lookup case-insensitive and should address the multipart failure with lowercaseetag:responses.Two minor follow-ups:
- The previous
strstrcall is now just commented out; it’s dead code and can be removed entirely to keep the function clean.strcasestris a non-standard extension; please confirm it’s properly declared and available on all supported platforms (e.g., via a common header or CMake feature check) so this doesn’t introduce build issues on non-GNU libc targets.
Prevents case sensitive ETag header names.
Fixes #11201
Testing
YAML config includes an S3 output stage to an S3 compatible service, and an HTTP input stage which we will use for testing:
To generate a large file to upload use the following script:
Before we can approve your change; please submit the following in a comment:
Valgrind report:
Backporting
Not required.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.