reitit

byrongibby 2025-02-27T14:26:37.391129Z

Hi there. I am wondering if someone might have a quick answer for me. When I enter certain random strings as query parameters instead of failing validation, as I would expect, the router simply drops the query parameters when parsing the query string. Is there a reason for this and there is there a way to get the router to rather return a validation error response in the cases where this happens?

byrongibby 2025-03-01T07:06:19.681409Z

Thanks again, I have created a issue for the community to discuss: https://github.com/ring-clojure/ring/issues/522

👍 1
opqdonut 2025-02-27T14:29:28.382439Z

whats the route definition?

opqdonut 2025-02-27T14:29:47.095879Z

are you using malli or something else for coercion?

byrongibby 2025-02-27T14:30:20.498609Z

I am using spec, I will get the route definition now

byrongibby 2025-02-27T14:34:28.297679Z

(s/def ::query-ref-4 (s/keys :opt-un [::agencyids
                                      ::ids
                                      ::versions
                                      ::relations]))

["/datasets"
 {:get {:tags ["Data sets"]
        :summary "Get data sets"
        :parameters {:query ::spec/query-ref-4}
        :handler (partial dataset/read-data-sets connection)}}]

byrongibby 2025-02-27T14:35:15.340189Z

(s/def ::ids (s/or :agencyids ::ids-type
                   :all ::sdmx/all))

byrongibby 2025-02-27T14:36:55.435759Z

(s/def ::ids-type (s/and string? #(re-matches comma-sep-ids-regex %)))

(def comma-sep-ids-regex (re-pattern (str "^(" sdmx/id-regex ")?(," sdmx/id-regex ")*$")))

(def id-regex #"[A-Z0-9](_?[A-Z0-9]+)*")

(s/def ::all (partial = "all"))

byrongibby 2025-02-27T14:37:36.665269Z

Do you think its a regex stuff up? If it is I don't understand why it is dropped when parsing the query string?

byrongibby 2025-02-27T14:38:16.754149Z

:data {:coercion reitit.coercion.spec/coercion
              :muuntaja muuntaja-sdmx-codera
              :middleware [parameters/parameters-middleware
                           muuntaja/format-negotiate-middleware
                           muuntaja/format-response-middleware
                           custom-exception-middleware
                           muuntaja/format-request-middleware
                           coercion/coerce-response-middleware
                           coercion/coerce-request-middleware
                           multipart/multipart-middleware]}

opqdonut 2025-02-27T14:43:51.756649Z

so an optional key gets dropped if it's value doesn't match the schema, hmm

opqdonut 2025-02-27T14:47:46.915209Z

that might just be how spec-tools coercion works, let me check

byrongibby 2025-02-27T14:50:02.011059Z

Thanks, much appreciated

opqdonut 2025-02-27T14:56:18.702599Z

no, I can't get the same behaviour just by playing around with st/coerce and st/conform

opqdonut 2025-02-27T14:56:23.694029Z

let me try to reproduce this more directly

byrongibby 2025-02-27T14:57:09.672519Z

I can also try reproduce a complete example if that would be useful?

opqdonut 2025-02-27T14:57:45.161269Z

yeah, that would help

byrongibby 2025-02-27T14:58:12.146579Z

Perfect, will post when I have something

juhoteperi 2025-02-27T16:51:17.575909Z

spec coercion uses strip-extra-keys-transformer by default: https://github.com/metosin/reitit/blob/master/modules/reitit-spec/src/reitit/coercion/spec.cljc#L25-L26

juhoteperi 2025-02-27T16:51:45.556999Z

ah but that is only for body params

juhoteperi 2025-02-27T16:52:02.233219Z

... string-transformer includes strip-extra-keys

byrongibby 2025-02-28T06:11:36.226289Z

Here is a small example https://github.com/byrongibby/reitit-test

byrongibby 2025-02-28T06:37:50.058739Z

Good morning @juhoteperi, to confirm, the observed behaviour in this case is the expected behaviour?

opqdonut 2025-02-28T06:45:05.173579Z

But this isn't an extra key in this case, it's specified in ::query-ref-4. Thanks for the example Byron, I'm looking into it now.

byrongibby 2025-02-28T06:45:21.607319Z

Thanks!

opqdonut 2025-02-28T06:56:59.286899Z

Yeah, I can reproduce the problem. Interesting! With

?agencyids=MY_AGENCY&ids=id_1 
I get "Request coercion failed", but with
?agencyids=MY_AGENCY&ids=%3c%%3d77%2a77%%3e
the coercion succeeds and :ids is missing from :query-params

byrongibby 2025-02-28T06:58:53.818899Z

Yep, it is strange, I wonder if this is what is intended

byrongibby 2025-02-28T06:59:12.201959Z

Thanks for reproducing!

opqdonut 2025-02-28T06:59:48.054199Z

I think this is caused by some earlier stage, not reitit coercion. Reitit coercion reads :query-params and writes into :parameters :query

byrongibby 2025-02-28T07:00:57.722269Z

Makes sense

opqdonut 2025-02-28T07:01:15.706089Z

and :query-params is missing the weird :ids even when I comment out

;;:coercion reitit.coercion.spec/coercion

opqdonut 2025-02-28T07:04:44.013619Z

reproduced:

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"}}

opqdonut 2025-02-28T07:05:10.938609Z

could the problem be that the garbage is not valid UTF-8 so it's being discarded?

opqdonut 2025-02-28T07:06:59.591429Z

perhaps this line in ring.util.codec is discarding the value: https://github.com/ring-clojure/ring-codec/blob/master/src/ring/util/codec.clj#L148

byrongibby 2025-02-28T07:07:51.939679Z

In your opinion this is not the behaviour that one would want right?

opqdonut 2025-02-28T07:08:44.734179Z

user=> (java.net.URLDecoder/decode "agencyids=MY_AGENCY&ids=%3c%%3d77%2a77%%3e" (java.nio.charset.Charset/forName "UTF-8"))
Execution error (IllegalArgumentException) at java.net.URLDecoder/decode (URLDecoder.java:237).
URLDecoder: Illegal hex characters in escape (%) pattern - Error at index 0 in: "%3"

opqdonut 2025-02-28T07:08:52.878269Z

that's the exception that gets swallowed

byrongibby 2025-02-28T07:09:28.970069Z

Aha

opqdonut 2025-02-28T07:09:50.141149Z

it's hard to say what's the right behaviour here, but yeah, I think I would prefer the params-middleware to return a HTTP 400 Bad Request in cases like this

opqdonut 2025-02-28T07:10:09.243449Z

you could consider filing a bug against ring

byrongibby 2025-02-28T07:10:34.804689Z

Thanks a lot for you help with this, I will think about it

opqdonut 2025-02-28T07:10:36.816449Z

bsless (the maintainer) might have some opinions on this

opqdonut 2025-02-28T07:10:48.797799Z

my message from above is a good repro for the bug: https://clojurians.slack.com/archives/C7YF1SBT3/p1740726284013619?thread_ts=1740666397.391129&cid=C7YF1SBT3

byrongibby 2025-02-28T07:11:01.693549Z

Great, thanks

juhoteperi 2025-03-02T12:15:21.134989Z

Did you encounter this with real use (ring-jetty-adapter, http-kit or such) or only when calling the handler function directly?

juhoteperi 2025-03-02T12:17:31.340069Z

And do http clients allow making such a request?

opqdonut 2025-03-03T06:06:40.252039Z

the data looked to me like it was generated by a security audit tool

byrongibby 2025-03-03T07:49:50.965129Z

@joel.kaasinen is exactly right, this issue came up during a penetration test. I can confirm that Firefox allows this request

byrongibby 2025-03-03T07:50:55.337499Z

The backend uses ring-jetty-adapter

byrongibby 2025-02-27T14:27:04.996329Z