Skip to content

Conversation

@dbu
Copy link
Contributor

@dbu dbu commented Nov 17, 2014

we discovered this in a rest api where a client was sending requests without accept headers. this is not good practice but allowed by the http spec. if the backend only has one type of answer, it usually works out.

we accidentally converted this into an existing Accept header with empty value, which by the HTTP spec means the client is not accepting any format. leading a spec-adherent backend with no other option than raising a 4xx error.

@ddeboer
Copy link
Member

ddeboer commented Nov 17, 2014

The things you find when using code in production. :)

Looks fine to me, but maybe add a test to ensure the correct behaviour?

CHANGELOG.md Outdated
Copy link
Member

Choose a reason for hiding this comment

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

This should be changed to 1.1.2 now.

@dbu dbu force-pushed the fix-accept-header-handling branch from d658e80 to bbdd2fe Compare November 21, 2014 11:40
@dbu
Copy link
Contributor Author

dbu commented Nov 21, 2014

There we go, added a test (i also tried breaking things and it actually works :-) test fails without the vcl fixes)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is just a weird hack to not need more backend php files for the testing - the permutations would lead to many files.

@ddeboer
Copy link
Member

ddeboer commented Nov 21, 2014

PR looks fine to me. Let’s add the .htaccess and then merge.

@dbu dbu force-pushed the fix-accept-header-handling branch from bbdd2fe to e894523 Compare November 21, 2014 13:03
ddeboer added a commit that referenced this pull request Nov 21, 2014
fix accept header handling when client sends no accept header
@ddeboer ddeboer merged commit 4e227b4 into master Nov 21, 2014
@ddeboer ddeboer deleted the fix-accept-header-handling branch November 21, 2014 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants