Merge pull request #115 from KingMob/url-decode-path-params

Add URL-decoding support for paths and params

modules/reitit-core/src/reitit/core.cljc

@@ -165,6 +165,23 @@
    :compile (fn [[_ {:keys [handler]}] _] handler)
    :conflicts (partial throw-on-conflicts! path-conflicts-str)})
 
+(defn- linear-router-lookup-structs
+  "Returns a 2-item vec of lookup structures.
+
+  The first is a vec of Routes.
+  The second is a map of route names to lookup fns."
+  [compiled-routes]
+  (reduce
+    (fn [[pl nl] [p {:keys [name] :as data} result]]
+      (let [{:keys [path-params] :as route} (impl/create [p data result])
+            f #(if-let [path (impl/path-for route %)]
+                 (->Match p data result % path)
+                 (->PartialMatch p data result % path-params))]
+        [(conj pl route)
+         (if name (assoc nl name f) nl)]))
+    [[] {}]
+    compiled-routes))
+
 (defn linear-router
   "Creates a linear-router from resolved routes and optional
   expanded options. See [[router]] for available options"
@@ -172,15 +189,7 @@
    (linear-router compiled-routes {}))
   ([compiled-routes opts]
    (let [names (find-names compiled-routes opts)
-         [pl nl] (reduce
+         [pl nl] (linear-router-lookup-structs compiled-routes)
-                   (fn [[pl nl] [p {:keys [name] :as data} result]]
-                     (let [{:keys [path-params] :as route} (impl/create [p data result])
-                           f #(if-let [path (impl/path-for route %)]
-                                (->Match p data result % path)
-                                (->PartialMatch p data result % path-params))]
-                       [(conj pl route)
-                        (if name (assoc nl name f) nl)]))
-                   [[] {}] compiled-routes)
          lookup (impl/fast-map nl)
          routes (uncompile-routes compiled-routes)]
      ^{:type ::router}
@@ -200,7 +209,7 @@
          (reduce
            (fn [_ ^Route route]
              (if-let [path-params ((:matcher route) path)]
-               (reduced (->Match (:path route) (:data route) (:result route) path-params path))))
+               (reduced (->Match (:path route) (:data route) (:result route) (impl/url-decode-coll path-params) path))))
            nil pl))
        (match-by-name [_ name]
          (if-let [match (impl/fast-get lookup name)]
@@ -209,6 +218,21 @@
          (if-let [match (impl/fast-get lookup name)]
            (match (impl/path-params path-params))))))))
 
+(defn- lookup-router-lookup-structs
+  "Returns a 2-item vec of lookup structures.
+
+  The first is a map of paths to Matches.
+  The second is a map of route names to Matches."
+  [compiled-routes]
+  (reduce
+    (fn [[pl nl] [p {:keys [name] :as data} result]]
+      [(assoc pl p (->Match p data result {} p))
+       (if name
+         (assoc nl name #(->Match p data result % p))
+         nl)])
+    [{} {}]
+    compiled-routes))
+
 (defn lookup-router
   "Creates a lookup-router from resolved routes and optional
   expanded options. See [[router]] for available options"
@@ -222,12 +246,7 @@
          {:wilds wilds
           :routes compiled-routes})))
    (let [names (find-names compiled-routes opts)
-         [pl nl] (reduce
+         [pl nl] (lookup-router-lookup-structs compiled-routes)
-                   (fn [[pl nl] [p {:keys [name] :as data} result]]
-                     [(assoc pl p (->Match p data result {} p))
-                      (if name
-                        (assoc nl name #(->Match p data result % p))
-                        nl)]) [{} {}] compiled-routes)
          data (impl/fast-map pl)
          lookup (impl/fast-map nl)
          routes (uncompile-routes compiled-routes)]
@@ -252,6 +271,23 @@
          (if-let [match (impl/fast-get lookup name)]
            (match (impl/path-params path-params))))))))
 
+(defn- segment-router-lookup-structs
+  "Returns a 2-item vec of lookup structures.
+
+  The first is a prefix-tree of segments and associated Matches.
+  The second is a map of route names to Matches or PartialMatches."
+  [compiled-routes]
+  (reduce
+    (fn [[pl nl] [p {:keys [name] :as data} result]]
+      (let [{:keys [path-params] :as route} (impl/create [p data result])
+            f #(if-let [path (impl/path-for route %)]
+                 (->Match p data result % path)
+                 (->PartialMatch p data result % path-params))]
+        [(segment/insert pl p (->Match p data result nil nil))
+         (if name (assoc nl name f) nl)]))
+    [nil {}]
+    compiled-routes))
+
 (defn segment-router
   "Creates a special prefix-tree style segment router from resolved routes and optional
   expanded options. See [[router]] for available options"
@@ -259,15 +295,7 @@
    (segment-router compiled-routes {}))
   ([compiled-routes opts]
    (let [names (find-names compiled-routes opts)
-         [pl nl] (reduce
+         [pl nl] (segment-router-lookup-structs compiled-routes)
-                   (fn [[pl nl] [p {:keys [name] :as data} result]]
-                     (let [{:keys [path-params] :as route} (impl/create [p data result])
-                           f #(if-let [path (impl/path-for route %)]
-                                (->Match p data result % path)
-                                (->PartialMatch p data result % path-params))]
-                       [(segment/insert pl p (->Match p data result nil nil))
-                        (if name (assoc nl name f) nl)]))
-                   [nil {}] compiled-routes)
          lookup (impl/fast-map nl)
          routes (uncompile-routes compiled-routes)]
      ^{:type ::router}
@@ -286,7 +314,7 @@
        (match-by-path [_ path]
          (if-let [match (segment/lookup pl path)]
            (-> (:data match)
-               (assoc :path-params (:path-params match))
+               (assoc :path-params (impl/url-decode-coll (:path-params match)))
                (assoc :path path))))
        (match-by-name [_ name]
          (if-let [match (impl/fast-get lookup name)]

modules/reitit-core/src/reitit/impl.cljc

@@ -19,6 +19,17 @@
               (java.util HashMap Map)
               (java.net URLEncoder URLDecoder))))
 
+(defn map-kv
+  "Applies a function to every value of a map.
+
+  Also works on vectors. Maintains key for maps, order for vectors."
+  [f coll]
+  (reduce-kv
+    (fn [m k v]
+      (assoc m k (f v)))
+    (empty coll)
+    coll))
+
 (defn wild? [s]
   (contains? #{\: \*} (first (str s))))
 
@@ -203,6 +214,11 @@
                s)
        :cljs (js/decodeURIComponent (str/replace s "+" " ")))))
 
+(defn url-decode-coll
+  "URL-decodes maps and vectors"
+  [coll]
+  (map-kv url-decode coll))
+
 (defprotocol IntoString
   (into-string [_]))
 
@@ -233,13 +249,9 @@
   (into-string [_]))
 
 (defn path-params
-  "shallow transform of the path parameters values into strings"
+  "Convert parameters' values into URL-encoded strings, suitable for URL paths"
   [params]
-  (reduce-kv
+  (map-kv #(url-encode (into-string %)) params))
-    (fn [m k v]
-      (assoc m k (url-encode (into-string v))))
-    {}
-    params))
 
 (defn query-string
   "shallow transform of query parameters into query string"
@@ -258,7 +270,7 @@
      (goog/inherits ~type ~base-type)
 
      ~@(map
-         (fn [method]
+        (fn [method]
-           `(set! (.. ~type -prototype ~(symbol (str "-" (first method))))
+          `(set! (.. ~type -prototype ~(symbol (str "-" (first method))))
-                  (fn ~@(rest method))))
+                 (fn ~@(rest method))))
-         methods)))
+        methods)))

modules/reitit-core/src/reitit/segment.cljc

@@ -38,7 +38,7 @@
              (segment children wilds catch-all match))))
        (-lookup [_ [p & ps] path-params]
          (if (nil? p)
-           (if match (assoc match :path-params path-params))
+           (when match (assoc match :path-params path-params))
            (or (-lookup (impl/fast-get children' p) ps path-params)
                (some #(-lookup (impl/fast-get children' %) ps (assoc path-params % p)) wilds)
                (-catch-all children' catch-all path-params p ps))))))))

test/cljc/reitit/core_test.cljc

@@ -51,6 +51,19 @@
                     #"^missing path-params for route /api/ipa/:size -> \#\{:size\}$"
                     (r/match-by-name! router ::beer))))))
 
+        (testing "decode %-encoded path params"
+          (let [router (r/router [["/one-param-path/:param1"]
+                                  ["/two-param-path/:param1/:param2"]
+                                  ["/catchall/*remaining-path"]] {:router r})
+                decoded-params #(-> router (r/match-by-path %) :path-params)
+                decoded-param1 #(-> (decoded-params %) :param1)
+                decoded-remaining-path #(-> (decoded-params %) :remaining-path)]
+            (is (= "foo bar" (decoded-param1 "/one-param-path/foo%20bar")))
+            (is (= {:param1 "foo bar" :param2 "baz qux"} (decoded-params "/two-param-path/foo%20bar/baz%20qux")))
+            (is (= "foo bar" (decoded-remaining-path "/catchall/foo%20bar")))
+            (is (= "!#$&'()*+,/:;=?@[]"
+                   (decoded-param1 "/one-param-path/%21%23%24%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D")))))
+
         (testing "complex"
           (let [router (r/router
                          [["/:abba" ::abba]