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

Malformed Accept-Language header causes server error (dropwizard-views) #2097

Closed
BenScholl opened this Issue Jul 7, 2017 · 5 comments

Comments

Projects
None yet
5 participants
@BenScholl
Contributor

BenScholl commented Jul 7, 2017

To reproduce:

curl -H 'Accept-Language:;' localhost:8080/some_view

This returns an HTTP 500 and dumps a stack trace to the app's server log.

It happens because HttpHeaders.getAcceptableLanguages(), as called by ViewMessageBodyWriter.detectLocale() can throw a HeaderValueException.

I think a "permissive" approach probably makes sense here: catch the exception and just fall back to the default Locale if the header is malformed.

@nickbabcock

This comment has been minimized.

Contributor

nickbabcock commented Jul 10, 2017

Your viewpoint makes sense. A server error should definitely not be the current behavior. Feel free to make a PR for it!

@FredDeschenes

This comment has been minimized.

Contributor

FredDeschenes commented Jul 12, 2017

I agree that this shouldn't return a server error, but I don't agree that the error should just be ignored and the default language used.

IMO, a 400 - Bad Request should be returned and the client should either not send the header, or send a properly formatted one.

FredDeschenes added a commit to FredDeschenes/dropwizard that referenced this issue Jul 12, 2017

Handle badly formed 'Accept-Language' headers (dropwizard#2097)
ViewMessageBodyWriter::detectLocale will now catch
`HeaderBalueException`s thrown when retrieving the languages from the
headers and return a `400 - Bad Request` to clients instead of a server
error.

FredDeschenes added a commit to FredDeschenes/dropwizard that referenced this issue Jul 12, 2017

Handle badly formed 'Accept-Language' headers (dropwizard#2097)
ViewMessageBodyWriter::detectLocale will now catch
`HeaderValueException`s thrown when retrieving the languages from the
headers and return a `400 - Bad Request` to clients instead of a server
error.
@BenScholl

This comment has been minimized.

Contributor

BenScholl commented Jul 12, 2017

I don't think it is a big deal either way.

However, just looking at the Alexa top 5:

$ curl -s -o /dev/null -w "%{http_code}\n" -H 'Accept-Language:;' http://www.google.co.uk
200

$ curl -s -o /dev/null -w "%{http_code}\n" -H 'Accept-Language:;' https://www.youtube.com
200

$ curl -s -o /dev/null -w "%{http_code}\n" -H 'Accept-Language:;' https://www.facebook.com
200

$ curl -s -o /dev/null -w "%{http_code}\n" -H 'Accept-Language:;' https://www.baidu.com
200

$ curl -s -o /dev/null -w "%{http_code}\n" -H 'Accept-Language:;' https://www.wikipedia.org
200
@FredDeschenes

This comment has been minimized.

Contributor

FredDeschenes commented Jul 12, 2017

Weird, I would've thought that most sites wouldn't let this go through.

I've also thought about having a configuration, but the views configuration is just a Map. We could have an overrideable function in "ViewBundle", but that's a bit weird. Also I think the feature is only available in Freemarker and not in Mustache, which makes this even trickier to configure.

arteam added a commit that referenced this issue Jul 27, 2017

Handle badly formed 'Accept-Language' headers (#2097) (#2103)
* Handle badly formed 'Accept-Language' headers (#2097)

ViewMessageBodyWriter::detectLocale will now catch
`HeaderValueException`s thrown when retrieving the languages from the
headers and return a `400 - Bad Request` to clients instead of a server
error.

* Add some more tests to ViewMessageBodyWriter
@arteam

This comment has been minimized.

Member

arteam commented Jul 27, 2017

Resolved by #2103

arteam added a commit that referenced this issue Jul 27, 2017

Handle badly formed 'Accept-Language' headers (#2097) (#2103)
* Handle badly formed 'Accept-Language' headers (#2097)

ViewMessageBodyWriter::detectLocale will now catch
`HeaderValueException`s thrown when retrieving the languages from the
headers and return a `400 - Bad Request` to clients instead of a server
error.

* Add some more tests to ViewMessageBodyWriter

@arteam arteam closed this Jul 27, 2017

@joschi joschi added this to the 1.1.3 milestone Aug 24, 2017

sankate pushed a commit to sankate/dropwizard that referenced this issue Nov 21, 2017

Handle badly formed 'Accept-Language' headers (dropwizard#2097) (drop…
…wizard#2103)

* Handle badly formed 'Accept-Language' headers (dropwizard#2097)

ViewMessageBodyWriter::detectLocale will now catch
`HeaderValueException`s thrown when retrieving the languages from the
headers and return a `400 - Bad Request` to clients instead of a server
error.

* Add some more tests to ViewMessageBodyWriter

aaanders added a commit to aaanders/dropwizard that referenced this issue Sep 20, 2018

Handle badly formed 'Accept-Language' headers (dropwizard#2097) (drop…
…wizard#2103)

* Handle badly formed 'Accept-Language' headers (dropwizard#2097)

ViewMessageBodyWriter::detectLocale will now catch
`HeaderValueException`s thrown when retrieving the languages from the
headers and return a `400 - Bad Request` to clients instead of a server
error.

* Add some more tests to ViewMessageBodyWriter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment