Skip to content
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

Error response is not parsed if Content-Type header also contains character set #3009

Closed
DetlefGolze opened this issue Jul 25, 2023 · 7 comments
Labels
bug This issue is a bug. pr/needs-review This PR needs a review from a Member. s3

Comments

@DetlefGolze
Copy link
Contributor

Describe the bug

An S3 store returns the following header in error responses:
Content-Type: application/xml;charset=utf-8

In this case the S3ErrorResponseUnmarshaller does not parse the XML body so that the produced exception does not contain full error information. Instead, the exception is filled only with data from the HTTP status, e.g.:

StatusCode: Forbidden
ErrorCode: Forbidden
Message: Error making request with Error Code Forbidden and Http Status Code Forbidden. No further error information was returned by the service.

Expected Behavior

Exception shall contain information from error response.

Current Behavior

Exception only uses HTTP status code.

Reproduction Steps

Please refer to error description above.

Possible Solution

Change line 86 in sdk\src\Services\S3\Custom\Model\Internal\MarshallTransformations\S3ErrorResponseUnmarshaller.cs

from

if (context.Stream.CanRead && contentLength != 0 && contentTypeHeader.EndsWith("/xml", stringComparison.OrdinalIgnoreCase))

to

if (context.Stream.CanRead && contentLength != 0 && (contentTypeHeader.EndsWith("/xml", StringComparison.OrdinalIgnoreCase) || contentTypeHeader.ToLower().Contains("/xml;")))

Additional Information/Context

No response

AWS .NET SDK and/or Package version used

AWSSDK.S3 version 3.7.201.2

Targeted .NET Platform

.NET 6

Operating System and version

Windows Server 2019

@DetlefGolze DetlefGolze added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 25, 2023
@ashishdhingra ashishdhingra added s3 needs-reproduction This issue needs reproduction. and removed needs-triage This issue or PR still needs to be triaged. labels Jul 25, 2023
@ashishdhingra ashishdhingra self-assigned this Jul 25, 2023
@ashishdhingra
Copy link
Contributor

@DetlefGolze Thanks for opening the issue. Could you please share the sample code to reproduce the issue where an S3 store returns Content-Type: application/xml;charset=utf-8 header in error responses? As an example, I tested with GetObjectAttributesAsync() operation, it returned the content type as application/xml in S3ErrorResponseUnmarshaller:

using (AmazonS3Client amazonS3Client = new AmazonS3Client(RegionEndpoint.USEast2))
{
    string objectName = "nosuchkey.txt";
    var request = new GetObjectAttributesRequest
    {
        BucketName = bucketName,
        Key = objectName,
        ObjectAttributes = new List<ObjectAttributes> { ObjectAttributes.ObjectSize }
    };
    GetObjectAttributesResponse response = await amazonS3Client.GetObjectAttributesAsync(request);

    Console.WriteLine(response.ObjectSize);
    Console.WriteLine(response.LastModified.ToString());
}

I understand the use case, however, it would be good to have a reproduction for further review with the team.

Thanks,
Ashish

@ashishdhingra ashishdhingra added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jul 25, 2023
@DetlefGolze
Copy link
Contributor Author

DetlefGolze commented Jul 26, 2023

@ashishdhingra Thanks for your effort and sorry for the confusion. I don't think that you can duplicate this behaviour with arbitrary S3 servers and especially not with AWS. I've seen this with another vendor's S3 server and as far as I can tell, it returns this header with any XML response.

I found this when using AmazonS3Util.DoesS3BucketExistV2Async() which relies on the S3 error code and unexpectedly throws due to the mentioned issue. Your code above would also show that issue.

I am not sure if I am allowed to post the name of the vendor here, but it is well known in the market. We will also inform them about this issue, but obviously it is not really a bug in their implementation.

@shadimari
Copy link

I was able to produce the issue using non AWS cloud vendor which has S3-compatabile object storage.
The content-type header is always return as Content-Type: application/xml;charset=utf-8
in which case the S3ErrorResponseUnmarshaller fails to parse the error response.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jul 27, 2023
@ashishdhingra ashishdhingra removed the needs-reproduction This issue needs reproduction. label Jul 28, 2023
@ashishdhingra
Copy link
Contributor

@DetlefGolze I saw you submitted PR #3011 for the fix. Could you please confirm if it works for your use case with non-AWS S3 (like) service?

@DetlefGolze
Copy link
Contributor Author

DetlefGolze commented Jul 28, 2023 via email

@ashishdhingra ashishdhingra removed their assignment Aug 3, 2023
@ashishdhingra ashishdhingra added the pr/needs-review This PR needs a review from a Member. label Aug 3, 2023
@ashishdhingra
Copy link
Contributor

The PR #3011 was released today in AWSSDK.S3 3.7.202 via commit deff03f.

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. pr/needs-review This PR needs a review from a Member. s3
Projects
None yet
Development

No branches or pull requests

3 participants