-
Notifications
You must be signed in to change notification settings - Fork 130
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
Handle malformed Accept-Language Accept-Charset & Accept-Encoding #299
Handle malformed Accept-Language Accept-Charset & Accept-Encoding #299
Conversation
Fixes clojure-liberator#199 Bad qvals in these headers will now cause a 400 status code to be returned.
Thanks for the patch, I will look into it soon. I wonder if 400 is the correct response for a malformed q-value. I think the http RFC suggests invalid values to be ignored, but I cannot think of a sensible default for an unparseable q-value: |
Any update on when this might be merged? |
ping! |
Hey @cch1, sorry for the delay. In RFC7231 we can read for the
We should prefer giving a valid response for an invalid Accept |
q (first (reverse (sort (filter (comp not nil?) | ||
(map #(let [[param value] (string/split % #"[\s\r\n]*=")] | ||
(if (= "q" param) (Float/parseFloat value))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try-catch-default to 0.01 here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds like a good, pragmatic solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ordnungswidrig Why 0.01? Is the idea to select a default value that is towards the bottom of the acceptable range for quality values, but leaves the actual minimum value (0.001) free in case the client correctly attaches it to some other entry?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, I'm not sure what the best would be in this case. The HTTP RFC suggest only to use three decimal places, so 0.001
is the smallest valid value. In the end this is just to make the server react as resilient as possible. If the client sends image/png;q=0.A;image/jpeg;q=0.1
then what would be a good response? We don't know of the client wanted to accept 0.1 for PNG or 0.9. But rather than sending no response I'd take the invalid one as the least acceptable for now.
@lverns looks good! Can you add an entry to |
Thanks for the patch! |
Handle malformed Accept-Language Accept-Charset & Accept-Encoding
Fixes #199
Bad qvals (either out of range, not parsable to a float) in these headers will now cause a 400 status code to be returned.
I'm not overly happy with this because we are sending a 400 status code, but not passing through
handle-malformed
which means users of this library can't change the rendering by overridinghandle-malformed
.The best they can do is check for the
:liberator.core/throwable
key on the response and add custom handling in middleware (e.g. ring), which may be good enough.