metosin / malli

High-performance data-driven data specification library for Clojure/Script.
Eclipse Public License 2.0
1.44k stars 205 forks source link

:decode/string for :uuid decodes truncated UUIDs in Clojure #867

Closed dmcgillen closed 1 year ago

dmcgillen commented 1 year ago

I hit this issue while using reitit to decode path parameters that contain UUIDs and I was checking what would happen if a supplied UUID path-param was invalid.

The underlying java.util.UUID/fromString does some weird things (IMHO!) when not given a full UUID, e.g.

(java.util.UUID/fromString "1-1-1-1-1")
;; => #uuid "00000001-0001-0001-0001-000000000001"

This means we get the following:

(malli.core/decode
 [:map [:id :uuid]]
 {:id "1-1-1-1-1"}
 malli.transform/string-transformer)
=> ;; {:id #uuid "00000001-0001-0001-0001-000000000001"}

and therefore:

(malli.core/validate
 [:map [:id :uuid]]
 (malli.core/decode
  [:map [:id :uuid]]
  {:id "1-1-1-1-1"}
  malli.transform/string-transformer))
;; => true

which I don't think is ever desired behaviour, and, for things like automatically parsing path parameters, is a bit of a problem.

We've worked around this in our code by replacing the :decode/string for :uuid to be something like:

(fn [string-uuid]
  (let [uuid (parse-uuid string-uuid)]
    (if (= string-uuid (str uuid))
      uuid
      string-uuid)))

i.e. we turn the uuid back to a string and make sure it matches. Didn't really put a lot of thought into a better way of doing it, but it works, at least!

I imagine a change something like the above should be made? Note that this problem doesn't arise in cljs as parse-uuid returns nil there. There seems to be quite a few issues raised against java.util.UUID/fromString but it seems like it's not going to be fixed there.

niwinz commented 1 year ago

And it is inconsistent with CLJS, where the full uuid is validated with regex. Just reported 5 min ago the same issue on clojurians slack.

niwinz commented 1 year ago

I will do a PR for fix it

ikitommi commented 1 year ago

Closed via #869