tailrecursion / ring-proxy

HTTP proxy ring middleware for Clojure web applications.
47 stars 14 forks source link

Two bugs in the way http-opts are merged #20

Open fhitchen opened 2 years ago

fhitchen commented 2 years ago

I use the proxy to add credentials or offload TLS client cert handling for http clients that don't support these things. Noticed two bugs recently.

1) The query parameter '?' was always being appended and newer versions of elasticsearch rejected the query. 2) If the http-options included a HTTP header, {:headers {:authorization "Bearer secret-token"}} for example, the request HTTP headers would get removed.

The solutions were to add logic to the :url processing and replace the merge function with the deep-merge function.

index 6a37c33..a2349b0 100644
--- a/src/tailrecursion/ring_proxy.clj
+++ b/src/tailrecursion/ring_proxy.clj
@@ -25,14 +24,19 @@
       (.read rdr buf)
       buf)))

+(defn deep-merge
+ "Recursively merges maps. If keys are not maps, the last value wins."
+ [& vals]
+ (if (every? map? vals)
+   (apply merge-with deep-merge vals)
+   (last vals)))
+
 (defn wrap-proxy
   "Proxies requests from proxied-path, a local URI, to the remote URI at
   remote-base-uri, also a string."
   [handler ^String proxied-path remote-base-uri & [http-opts]]
   (wrap-cookies
    (fn [req]
      (if (.startsWith ^String (:uri req) proxied-path)
        (let [rmt-full   (URI. (str remote-base-uri "/"))
              rmt-path   (URI. (.getScheme    rmt-full)
@@ -40,8 +44,8 @@
                               (.getPath      rmt-full) nil nil)
              lcl-path   (URI. (subs (:uri req) (.length proxied-path)))
              remote-uri (.resolve rmt-path lcl-path) ]
-         (-> (merge {:method (:request-method req)
-                     :url (str remote-uri "?" (:query-string req))
+         (-> (deep-merge {:method (:request-method req)
+                     :url (str remote-uri (if (nil? (:query-string req)) "" "?") (:query-string req))
                      :headers (dissoc (:headers req) "host" "content-length")
                      :body (if-let [len (get-in req [:headers "content-length"])]
                              (slurp-binary (:body req) (Integer/parseInt len)))