metosin / reitit

A fast data-driven routing library for Clojure/Script
https://cljdoc.org/d/metosin/reitit/
Eclipse Public License 1.0
1.42k stars 255 forks source link

Header coercion no longer works using aleph with 0.7.2 #700

Open valerauko opened 1 week ago

valerauko commented 1 week ago

Might be a regression introduced by #506 ?

aleph headers are potemkin magical maps where keys can be accessed both as strings and as keywords.

With a reitit route that has :parameters {:header {:authorization (,,, spec-tools spec for Bearer ,,,)}} and :coercion spec-coercion/coercion, with 0.7.1 coercion/coerce-request-middleware works as expected and valid requests pass.

However with 0.7.2 this no longer works failing with "Request coercion failed". In 0.7.2 if you add a middleware like below it once again works as expected, so I suspected it might be some optimizations in #506 clashing with aleph's (potemkin's) custom map types.

(fn [handler]
  (fn [request]
    (handler (update request :headers #(into {} %)))))
ikitommi commented 1 week ago

@bsless, ideas on this?

bsless commented 1 week ago

I was afraid of this, where someone might get clever with types and I missed it in my extend / dispatch. I'll pull Aleph in and check what I missed

valerauko commented 1 week ago

@bsless here are some relevant bits

https://github.com/clj-commons/aleph/blob/cd42a65980af60c138a10be31b63e8ffeb8f8712/src/aleph/http/core.clj#L108 https://github.com/clj-commons/potemkin/blob/4407a358eec50fd42bfdc6637f0b1624c7d3d2cf/src/potemkin/collections.clj#L206

bsless commented 1 week ago

@valerauko I have a fix and I tested it locally with Aleph headers and got the expected results, but if you could check out the patch and test it with your code it will be additional verification of this fix's correctness

valerauko commented 1 week ago

No problem, but I won't be able to check until Wednesday

bsless commented 9 hours ago

@valerauko any update?