Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
fhitchen opened this issue Feb 13, 2022 · 0 comments
Open

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

fhitchen opened this issue Feb 13, 2022 · 0 comments

Comments

@fhitchen
Copy link

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)))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant