Merge pull request #23 from ddellacosta/master

provide a non-decoded :path-info to ring
immutant · Mar 2, 2016 · de6dd51 · de6dd51
2 parents eea773b + 5343fbd
commit de6dd51
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 5 deletions.
diff --git a/web/src/immutant/web/internal/servlet.clj b/web/src/immutant/web/internal/servlet.clj
@@ -13,7 +13,8 @@
 ;; limitations under the License.
 
 (ns ^{:no-doc true} immutant.web.internal.servlet
-  (:require [immutant.web.internal.ring :as ring]
+  (:require [clojure.string :as str]
+            [immutant.web.internal.ring :as ring]
             [immutant.web.internal.headers :as hdr]
             [immutant.internal.util :refer [try-resolve warn]]
             [immutant.util :refer [in-eap?]]
@@ -68,6 +69,27 @@
   (set-expiry [session timeout]
     (.setMaxInactiveInterval session timeout)))
 
+;;
+;; V1 comment:
+;; We don't use .getPathInfo (HttpServletRequest) since it is
+;; decoded. See IMMUTANT-195.
+;;
+;; ---
+;; This function ported from V1.x, also see IMMUTANT-610
+;; See also path-info' in immutant.web.internal.undertow. We un-DRYed
+;; this to avoid the cost associated with reflection on the request
+;; arg.
+;;
+(defn- path-info'
+  "Takes a HttpServletRequest and returns a path-info string without
+  any URL decoding performed upon it."
+  [^HttpServletRequest request]
+  (let [path-info (subs (.getRequestURI request)
+                        (count (ring/context request)))]
+    (if (str/blank? path-info)
+      "/"
+      path-info)))
+
 (extend-type HttpServletRequest
   ring/RingRequest
   (server-port [request]        (.getServerPort request))
@@ -83,7 +105,7 @@
   (headers [request]            (hdr/headers->map request))
   (body [request]               (.getInputStream request))
   (context [request]            (str (.getContextPath request) (.getServletPath request)))
-  (path-info [request]          (or (.getPathInfo request) "/"))
+  (path-info [request]          (path-info' request))
   (ssl-client-cert [request]    (first (.getAttribute request "javax.servlet.request.X509Certificate")))
   hdr/Headers
   (get-names [request]      (enumeration-seq (.getHeaderNames request)))

diff --git a/web/src/immutant/web/internal/undertow.clj b/web/src/immutant/web/internal/undertow.clj
@@ -13,7 +13,8 @@
 ;; limitations under the License.
 
 (ns ^{:no-doc true} immutant.web.internal.undertow
-    (:require [immutant.web.async            :as async]
+    (:require [clojure.string                :as str]
+              [immutant.web.async            :as async]
               [immutant.web.internal.headers :as hdr]
               [immutant.web.internal.ring    :as ring]
               [ring.middleware.session       :as ring-session])
@@ -99,6 +100,27 @@
   (set-header [headers ^String k ^String v] (.put headers (HttpString. k) v))
   (add-header [headers ^String k ^String v] (.add headers (HttpString. k) v)))
 
+;;
+;; V1 comment:
+;; We don't use .getRequestPath (HttpServerExchange) since it is
+;; decoded. See IMMUTANT-195.
+;;
+;; ---
+;; This function ported from V1.x, also see IMMUTANT-610
+;; See also path-info' in immutant.web.internal.servlet. Un-DRYed
+;; this to avoid the cost associated with reflection on the request
+;; arg.
+;;
+(defn- path-info'
+  "Takes a HttpServerExchange and returns a path-info string without
+  any URL decoding performed upon it."
+  [^HttpServerExchange request]
+  (let [path-info (subs (.getRequestURI request)
+                        (count (ring/context request)))]
+    (if (str/blank? path-info)
+      "/"
+      path-info)))
+
 (extend-type HttpServerExchange
   ring/RingRequest
   (server-port [exchange]        (-> exchange .getDestinationAddress .getPort))
@@ -115,8 +137,7 @@
   (headers [exchange]            (-> exchange .getRequestHeaders hdr/headers->map))
   (body [exchange]               (when (.isBlocking exchange) (.getInputStream exchange)))
   (context [exchange]            (.getResolvedPath exchange))
-  (path-info [exchange]          (let [v (.getRelativePath exchange)]
-                                   (if (empty? v) "/" v)))
+  (path-info [exchange]          (path-info' exchange))
   (ssl-client-cert [_])
 
   ring/RingResponse

diff --git a/web/test-integration/immutant/web/integ_test.clj b/web/test-integration/immutant/web/integ_test.clj
@@ -125,6 +125,12 @@
     (is (map? (:headers request)))
     (is (< 3 (count (:headers request))))))
 
+;; IMMUTANT-610
+(marktest test-non-decoded-path-info
+  (let [request (decode (get-body (str (url) "dump/request%2B?query=help")
+                                  :headers {:content-type "text/html; charset=utf-8"}))]
+    (is (= (:path-info request) "/request%2B"))))
+
 (marktest non-existent-query-string-is-nil
   (let [request (decode (get-body (str (url) "dump/request") :headers {:content-type "text/html; charset=utf-8"}))]
     (is (nil? (:query-string request)))))