Skip to content
This repository has been archived by the owner on Mar 22, 2023. It is now read-only.

Commit

Permalink
adds support for protocol specific hop-by-hop header removal
Browse files Browse the repository at this point in the history
  • Loading branch information
@shamsimam
shamsimam committed Apr 25, 2019
1 parent 4ccf0fc commit 45113c1
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 18 deletions.
18 changes: 13 additions & 5 deletions waiter/src/waiter/headers.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,24 @@
(assoc truncated-headers "x-waiter-token" token)
truncated-headers)))

(defn- retrieve-proto-specific-hop-by-hop-headers
"Determines the protocol version specific hop-by-hop headers."
[proto-version]
(if (= "HTTP/2.0" proto-version) [] ["te"]))

(defn dissoc-hop-by-hop-headers
"Proxies must remove hop-by-hop headers before forwarding messages — both requests and responses.
Remove the hop-by-hop headers (except te) specified in:
https://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.5.1
The te header is not removed as it is needed by grpc to detect incompatible proxies:
The te header is not removed for HTTP/2.0 requests as it is needed by grpc to detect incompatible proxies:
https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md"
[{:strs [connection] :as headers}]
(let [connection-headers (map str/trim (str/split (str connection) #","))]
(apply dissoc headers "connection" "keep-alive" "proxy-authenticate" "proxy-authorization"
"trailers" "transfer-encoding" "upgrade" connection-headers)))
[{:strs [connection] :as headers} proto-version]
(let [force-remove-headers (retrieve-proto-specific-hop-by-hop-headers proto-version)
connection-headers (map str/trim (str/split (str connection) #","))]
(cond-> (dissoc headers "connection" "keep-alive" "proxy-authenticate" "proxy-authorization"
"trailers" "transfer-encoding" "upgrade")
(seq force-remove-headers) (as-> $ (apply dissoc $ force-remove-headers))
(seq connection-headers) (as-> $ (apply dissoc $ connection-headers)))))

(defn assoc-auth-headers
"`assoc`s the x-waiter-auth-principal and x-waiter-authenticated-principal headers if the
Expand Down
4 changes: 2 additions & 2 deletions waiter/src/waiter/process_request.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@
; Removing expect may be dangerous http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html, but makes requests 3x faster =}
; Also remove hop-by-hop headers https://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.5.1
headers (-> (dissoc passthrough-headers "authorization" "expect")
(headers/dissoc-hop-by-hop-headers)
(headers/dissoc-hop-by-hop-headers proto-version)
(assoc "cookie" (auth/remove-auth-cookie (get passthrough-headers "cookie"))))
waiter-debug-enabled? (utils/request->debug-enabled? request)]
(try
Expand Down Expand Up @@ -500,7 +500,7 @@
(catch Exception e
(async/close! request-state-chan)
(handle-process-exception e request)))
(update :headers headers/dissoc-hop-by-hop-headers)
(update :headers headers/dissoc-hop-by-hop-headers proto-version)
(assoc :get-instance-latency-ns instance-elapsed
:instance instance
:protocol proto-version)
Expand Down
4 changes: 2 additions & 2 deletions waiter/src/waiter/websocket.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,12 @@

(defn make-request
"Makes an asynchronous websocket request to the instance endpoint and returns a channel."
[websocket-client service-id->password-fn {:keys [host port] :as instance} ws-request request-properties passthrough-headers end-route _ backend-proto _]
[websocket-client service-id->password-fn {:keys [host port] :as instance} ws-request request-properties passthrough-headers end-route _ backend-proto proto-version]
(let [ws-middleware (fn ws-middleware [_ ^UpgradeRequest request]
(let [service-password (-> instance scheduler/instance->service-id service-id->password-fn)
headers
(-> (dissoc passthrough-headers "content-length" "expect" "authorization")
(headers/dissoc-hop-by-hop-headers)
(headers/dissoc-hop-by-hop-headers proto-version)
(dissoc-forbidden-headers)
(assoc "Authorization" (str "Basic " (String. ^bytes (b64/encode (.getBytes (str "waiter:" service-password) "utf-8")) "utf-8")))
(headers/assoc-auth-headers (:authorization/user ws-request) (:authorization/principal ws-request))
Expand Down
8 changes: 7 additions & 1 deletion waiter/test/waiter/headers_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,4 +172,10 @@
"proxy-connection" "keep-alive"
"referer" "http://www.test-referer.com"
"te" "trailers, deflate"}
(dissoc-hop-by-hop-headers headers)))))
(dissoc-hop-by-hop-headers headers "HTTP/2.0")))
(is (= {"bar" "bar-value"
"content" "text/html"
"content-encoding" "gzip"
"proxy-connection" "keep-alive"
"referer" "http://www.test-referer.com"}
(dissoc-hop-by-hop-headers headers "HTTP/1.0")))))
26 changes: 18 additions & 8 deletions waiter/test/waiter/process_request_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,35 +315,45 @@
service-id->password-fn (fn service-id->password-fn [service-id]
(is (= "test-service-id" service-id))
app-password)
proto-version "HTTP/1.0"
http-clients {:http1-client (http/client)}
http-request-mock-factory (fn [passthrough-headers request-method-fn-call-counter]
http-request-mock-factory (fn [passthrough-headers request-method-fn-call-counter proto-version]
(fn [^HttpClient _ request-config]
(swap! request-method-fn-call-counter inc)
(is (= expected-endpoint (:url request-config)))
(is (= :bytes (:as request-config)))
(is (:auth request-config))
(is (= "body" (:body request-config)))
(is (= 654321 (:idle-timeout request-config)))
(is (= (-> (dissoc passthrough-headers "expect" "authorization"
(is (= (-> (apply dissoc passthrough-headers (if (= proto-version "HTTP/2.0") [] ["te"]))
(dissoc "expect" "authorization"
"connection" "keep-alive" "proxy-authenticate" "proxy-authorization"
"trailers" "transfer-encoding" "upgrade")
(merge {"x-waiter-auth-principal" "test-user"
"x-waiter-authenticated-principal" "[email protected]"}))
(:headers request-config)))
(is (= proto-version (:version request-config)))))]
(testing "make-request:headers"
(let [request-method-fn-call-counter (atom 0)]
(with-redefs [http/request (http-request-mock-factory passthrough-headers request-method-fn-call-counter)]
(testing "make-request:headers:HTTP/1.0"
(let [proto-version "HTTP/1.0"
request-method-fn-call-counter (atom 0)]
(with-redefs [http/request (http-request-mock-factory passthrough-headers request-method-fn-call-counter proto-version)]
(make-request http-clients make-basic-auth-fn service-id->password-fn instance request request-properties
passthrough-headers end-route nil backend-proto proto-version)
(is (= 1 @request-method-fn-call-counter)))))

(testing "make-request:headers:HTTP/2.0"
(let [proto-version "HTTP/2.0"
request-method-fn-call-counter (atom 0)]
(with-redefs [http/request (http-request-mock-factory passthrough-headers request-method-fn-call-counter proto-version)]
(make-request http-clients make-basic-auth-fn service-id->password-fn instance request request-properties
passthrough-headers end-route nil backend-proto proto-version)
(is (= 1 @request-method-fn-call-counter)))))

(testing "make-request:headers-long-content-length"
(let [request-method-fn-call-counter (atom 0)
(let [proto-version "HTTP/1.0"
request-method-fn-call-counter (atom 0)
passthrough-headers (assoc passthrough-headers "content-length" "1234123412341234")
statsd-inc-call-value (promise)]
(with-redefs [http/request (http-request-mock-factory passthrough-headers request-method-fn-call-counter)
(with-redefs [http/request (http-request-mock-factory passthrough-headers request-method-fn-call-counter proto-version)
statsd/inc!
(fn [metric-group metric value]
(is (nil? metric-group))
Expand Down

0 comments on commit 45113c1

Please sign in to comment.