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

More Specific Response Content Type #198

Merged
merged 1 commit into from Dec 28, 2019
Merged

More Specific Response Content Type #198

merged 1 commit into from Dec 28, 2019

Conversation

an-tex
Copy link
Contributor

@an-tex an-tex commented Nov 27, 2019

Description

Some of the S3Mock endoints return a application/x-www-form-urlencoded Content-Type, which is ignored by most clients but is HTTP specification wise not correct. The AWS API spec doesn't explicitly describe the Content-Type, but the the actual response body is XML and the Content-Type application/octet-stream

Some strict clients (e.g. Akka HTTP, Alpakka), correctly reject parsing a Content-Type of application/x-www-form-urlencoded as XML, but rather expect application/octet-stream or application/xml.

I've tried changing the Content-Type to application/octet-stream, but the Spring Framework always returns a 400 - Bad Request then in the IT tests. I couldn't figure out why, but my guess is that it doesn't know that this response should be marshalled XML. Changing the Content-Type to return application/xml works on the other hand. Both, the IT tests and the Akka HTTP client are happy then. I know that's not ideal as it's not an accurate implementation of the S3 API, but I'd argue it is still more correct than the current implementation.

Related Issue

#35

Tasks

  • I have signed the CLA.
  • I have written tests and verified that they fail without my change.

@timoe
Copy link
Contributor

timoe commented Nov 29, 2019

Thanks for the contribution, let me take a look.

@DanieleSassoli
Copy link

hey guys, any update on this? I can't really tell why the build is failing on travis, probably missing something.
@an-tex are you able to understand what's going on or do you need a hand? I'd be happy to have a look into this if it helps, but I think you know far better than me what needs doing.

@ChaithanyaGK
Copy link
Contributor

@DanieleSassoli Build is failing due to check-style violation https://travis-ci.org/adobe/S3Mock/builds/619544525#L575

@an-tex
Copy link
Contributor Author

an-tex commented Dec 28, 2019

Thanks for the hint @DanieleSassoli @ChaithanyaGK . I missed to follow up here. Just pushed the checkstyle changes incl. merge from upstream master

@an-tex an-tex requested a review from timoe December 28, 2019 09:03
Copy link
Contributor

@timoe timoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

squash commits pls

…esponse content type to be more consistent with http and aws spec
@timoe timoe merged commit 5a68061 into adobe:master Dec 28, 2019
@timoe
Copy link
Contributor

timoe commented Dec 28, 2019

I'll do a release on Monday

@timoe
Copy link
Contributor

timoe commented Dec 30, 2019

2.1.18 was released

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.

None yet

4 participants