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

avoids clearing TE headers while sending requests to the backend #708

Merged
merged 2 commits into from
Apr 25, 2019
Merged

avoids clearing TE headers while sending requests to the backend #708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4ccf0fc
avoids clearing TE headers while sending requests to the backend
shamsimam Apr 18, 2019
f44502e
adds support for protocol specific hop-by-hop header removal
shamsimam Apr 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions waiter/src/waiter/headers.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +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]
(when (not= "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 as specified in https://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.5.1"
[{:strs [connection] :as headers}]
(let [connection-headers (map str/trim (str/split (str connection) #","))]
(apply dissoc headers "connection" "keep-alive" "proxy-authenticate" "proxy-authorization"
"te" "trailers" "transfer-encoding" "upgrade" connection-headers)))
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 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} 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
Open in desktop
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
Open in desktop
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
9 changes: 8 additions & 1 deletion waiter/test/waiter/headers_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,16 @@
"te" "trailers, deflate"
"transfer-encoding" "trailers, deflate"
"upgrade" "http/2.0, https/1.3, irc/6.9, rta/x11, websocket"}]
(is (= {"bar" "bar-value"
"content" "text/html"
"content-encoding" "gzip"
"proxy-connection" "keep-alive"
"referer" "http://www.test-referer.com"
"te" "trailers, deflate"}
(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)))))
(dissoc-hop-by-hop-headers headers "HTTP/1.0")))))
28 changes: 19 additions & 9 deletions waiter/test/waiter/process_request_test.clj
View file
Open in desktop
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"
"te" "trailers" "transfer-encoding" "upgrade")
"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