[v25.1.x] cloud_storage_clients: check Content-Type header before parsing REST error responses in s3_client
#25817
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`ss::httpd::function_handler` calls `rep->done(_type)` on the reply before returning it. Because certain code paths in imposter services may want to return a different `Content-Type` in their response, this impl's `handle()` function allows for modifying the `type` set in the function handler's body before the final call to `done(type)` is performed. This class also handles an oversight in `mime_types.cc`, in which `xml` is not handled as a possible `mime` type. (cherry picked from commit 7477d92)
All S3 error codes should be returned using `application/xml` as the `Content-Type` header. (cherry picked from commit 0f6d14a)
There are code paths here that possibly return a `xml` body for their response. They need to ensure the response will be properly marked with `application/xml` in the `Content-Type` header. (cherry picked from commit 8dd0922)
…ly_with()` Sometimes we need to be able to set the headers and the body of a response. Add a new overload that allows us to do so. (cherry picked from commit a803a63)
…tor_test` The `not_found_response` is XML encoded. (cherry picked from commit 115bf81)
…t_test` More XML encoded responses that need their `Content-Type` headers properly set. (cherry picked from commit 366027f)
Future commits will add use of this function to the `s3_client`. For that reason, move the `enum class response_content_type` to `types.h`, and `get_response_content_type()` helper function to `util.cc/.h`. (cherry picked from commit cc5b49f)
According to the S3 docs (`https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html`), REST error responses _should_ have the Content-Type header set to `application/xml`. However, response like `503 Service Unavailable` may not be of this assumed `Content-Type`. As seen in CI (abridged output): ``` WARN s3 - s3_client.cc:841 - S3 PUT request failed for key "...": Service Unavailable [Content-Type: text/plain; charset=utf-8]; ... ERROR s3 - util.cc:209 - !!parsing error boost::wrapexcept<boost::property_tree::xml_parser::xml_parser_error> (<unspecified file>(1): expected <) ERROR s3 - s3_client.cc:441 - !!error parse error boost::wrapexcept<boost::property_tree::xml_parser::xml_parser_error> (<unspecified file>(1): expected <) ``` To fix this issue, read the `Content-Type` header in `s3_client` instead of assuming `xml` as the `Content-Type` and attempting to parse it. If the `Content-Type` is not `xml`, we will return a partially filled `rest_error_response`. (cherry picked from commit 1cc76c7)
WillemKauf
approved these changes
Apr 15, 2025
CI test resultstest results on build#64559
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of PR #25738