Add URL-decoding support

Enables matching URLs with %-encoded chars, and decodes path params.
Currently Reitit cannot handle %-encoded paths, even though they can be
quite common (like `/search/my random search string`)

E.g.:
Successfully match `/space in path` to `/space%20in%20path`
Match `/path/:param1` to `/path/foo%20bar` and see
`:param1` => `"foo bar"` in path params

Does not apply %-decoding to routes, only URLs
Does not guarantee decoding '+' into a space. (Java's URLDecoder
supports it, but it shouldn't for URL path part.)
This commit is contained in:
Matthew Davidson 2018-07-23 00:45:50 -04:00
parent 48509fefbd
commit f6054a7c40
4 changed files with 47 additions and 18 deletions

View file

@ -241,7 +241,7 @@
(route-names [_] (route-names [_]
names) names)
(match-by-path [_ path] (match-by-path [_ path]
(impl/fast-get data path)) (impl/fast-get data (impl/url-decode path)))
(match-by-name [_ name] (match-by-name [_ name]
(if-let [match (impl/fast-get lookup name)] (if-let [match (impl/fast-get lookup name)]
(match nil))) (match nil)))
@ -321,7 +321,7 @@
(route-names [_] (route-names [_]
names) names)
(match-by-path [_ path] (match-by-path [_ path]
(if (#?(:clj .equals :cljs =) p path) (if (#?(:clj .equals :cljs =) p (impl/url-decode path))
match)) match))
(match-by-name [_ name] (match-by-name [_ name]
(if (= n name) (if (= n name)

View file

@ -45,6 +45,17 @@
(defn contains-wilds? [path] (defn contains-wilds? [path]
(boolean (some wild-or-catch-all-param? (segments path)))) (boolean (some wild-or-catch-all-param? (segments path))))
(defn url-encode [s]
(some-> s
#?(:clj (URLEncoder/encode "UTF-8")
:cljs (js/encodeURIComponent))
#?(:clj (.replace "+" "%20"))))
(defn url-decode [s]
(some-> s #?(:clj (URLDecoder/decode "UTF-8")
:cljs (js/decodeURIComponent))))
;; ;;
;; https://github.com/pedestal/pedestal/blob/master/route/src/io/pedestal/http/route/path.clj ;; https://github.com/pedestal/pedestal/blob/master/route/src/io/pedestal/http/route/path.clj
;; ;;
@ -93,7 +104,7 @@
(let [{:keys [path-re path-params]} route] (let [{:keys [path-re path-params]} route]
(fn [path] (fn [path]
(when-let [m (re-matches path-re path)] (when-let [m (re-matches path-re path)]
(zipmap path-params (rest m)))))) (zipmap path-params (map url-decode (rest m)))))))
;; ;;
;; Routing (c) Metosin ;; Routing (c) Metosin
@ -108,7 +119,7 @@
(merge $ {:path path (merge $ {:path path
:matcher (if (contains-wilds? path) :matcher (if (contains-wilds? path)
(path-matcher $) (path-matcher $)
#(if (#?(:clj .equals, :cljs =) path %) {})) #(if (#?(:clj .equals, :cljs =) path (url-decode %)) {}))
:result result :result result
:data data}) :data data})
(dissoc $ :path-re :path-constraints) (dissoc $ :path-re :path-constraints)
@ -166,16 +177,6 @@
;; Path-parameters, see https://github.com/metosin/reitit/issues/75 ;; Path-parameters, see https://github.com/metosin/reitit/issues/75
;; ;;
(defn url-encode [s]
(some-> s
#?(:clj (URLEncoder/encode "UTF-8")
:cljs (js/encodeURIComponent))
#?(:clj (.replace "+" "%20"))))
(defn url-decode [s]
(some-> s #?(:clj (URLDecoder/decode "UTF-8")
:cljs (js/decodeURIComponent))))
(defprotocol IntoString (defprotocol IntoString
(into-string [_])) (into-string [_]))

View file

@ -39,9 +39,10 @@
(-lookup [_ [p & ps] path-params] (-lookup [_ [p & ps] path-params]
(if (nil? p) (if (nil? p)
(if match (assoc match :path-params path-params)) (if match (assoc match :path-params path-params))
(let [p (impl/url-decode p)]
(or (-lookup (impl/fast-get children' p) ps path-params) (or (-lookup (impl/fast-get children' p) ps path-params)
(some #(-lookup (impl/fast-get children' %) ps (assoc path-params % p)) wilds) (some #(-lookup (impl/fast-get children' %) ps (assoc path-params % p)) wilds)
(-catch-all children' catch-all path-params p ps)))))))) (-catch-all children' catch-all path-params p ps)))))))))
(defn insert [root path data] (defn insert [root path data]
(-insert (or root (segment)) (impl/segments path) (map->Match {:data data}))) (-insert (or root (segment)) (impl/segments path) (map->Match {:data data})))

View file

@ -51,6 +51,22 @@
#"^missing path-params for route /api/ipa/:size -> \#\{:size\}$" #"^missing path-params for route /api/ipa/:size -> \#\{:size\}$"
(r/match-by-name! router ::beer)))))) (r/match-by-name! router ::beer))))))
(testing "URL-encoded"
(let [router (r/router [["/one-param-path/:param"]
["/two-param-path/:param1/:param2"]
["/catchall/*remaining-path"]
["/space in path"]] {:router r})
decoded-params #(-> router (r/match-by-path %) :path-params)
decoded-param #(-> (decoded-params %) :param)
decoded-catchall #(-> (decoded-params %) :remaining-path)]
(println "Testing" (r/router-name router))
(is (= "foo bar" (decoded-param "/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-catchall "/catchall/foo%20bar")))
(is (= "foo bar" (decoded-catchall "/catchall/foo%20bar")))
(is (= "!#$&'()*+,/:;=?@[]" (decoded-param "/one-param-path/%21%23%24%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D")))
(is (= "/space in path" (-> router (r/match-by-path "/space%20in%20path") :template)))))
(testing "complex" (testing "complex"
(let [router (r/router (let [router (r/router
[["/:abba" ::abba] [["/:abba" ::abba]
@ -100,7 +116,18 @@
#"can't create :lookup-router with wildcard routes" #"can't create :lookup-router with wildcard routes"
(r/lookup-router (r/lookup-router
(r/resolve-routes (r/resolve-routes
["/api/:version/ping"] {})))))) ["/api/:version/ping"] {})))))
(testing "URL-decoding"
(let [router (r/router [["/space in path"]] {:router r})
matched-template #(-> router (r/match-by-path %) :template)]
(is (= "/space in path" (matched-template "/space%20in%20path"))))
(testing "should only apply to real URLs, not configured routes"
(let [router (r/router [["/percent%20in%20path"]] {:router r})
matched-template #(-> router (r/match-by-path %) :template)]
(is (= "/percent%20in%20path" (matched-template "/percent%2520in%2520path")))
(is (not= "/percent%20in%20path" (matched-template "/percent%20in%20path")))))))
r/lookup-router :lookup-router r/lookup-router :lookup-router
r/single-static-path-router :single-static-path-router r/single-static-path-router :single-static-path-router