Potentially incorrect behaviour processing URL query parameters

[byrongibby]

When an error is raised while processing query parameters the offending query parameter is dropped from the response

user=> (require '[ring.middleware.params :as p])
nil

user=> (p/assoc-query-params {:query-string "agencyids=MY_AGENCY&ids=ID_1,ID_2"} "UTF-8")
{:query-string "agencyids=MY_AGENCY&ids=ID_1,ID_2", :query-params {"agencyids" "MY_AGENCY", "ids" "ID_1,ID_2"}, :params {"agencyids" "MY_AGENCY", "ids" "ID_1,ID_2"}}

user=> (p/assoc-query-params {:query-string "agencyids=MY_AGENCY&ids=%3c%%3d77%2a77%%3e"} "UTF-8")
{:query-string "agencyids=MY_AGENCY&ids=%3c%%3d77%2a77%%3e", :query-params {"agencyids" "MY_AGENCY"}, :params {"agencyids" "MY_AGENCY"}}

In the last line the ids parameter is dropped as it contains illegal characters %3c%%3d77%2a77%%3e and cannot be decoded by java.net.URLDecoder/decode.

The downstream effect of this is that the query proceeds and ultimately responds with with a 200 OK while I would expect that you would want a 400 Bad Request response in this case. I have not been able to find an authoritative source on the correct response and I am interested to hear any opinions on this?

[weavejester]

It's currently the responsibility of the adapter to weed out invalid HTTP requests, and the above encoding would appear to violate the percent encoding scheme of URLs.

That said, as far as I can tell UTF-8 is not mandated as the encoding scheme for percent encoding in the query string, merely recommended, so it may be possible that a URL may be valid but undecodable. Perhaps a solution would be to add an :decoding-error-response option to wrap-params , that would be returned if the decoding has an error. If the option is not set, the current behavior is maintained.

[byrongibby]

Hi. Thank you very much for the helpful response. I can work on a PR to that effect if it is agreed that that would be helpful.

[weavejester]

Sorry about the delayed response. Yes, a PR would be appreciated.