diff --git a/waiter/src/waiter/headers.clj b/waiter/src/waiter/headers.clj index 791a61642..d44ed797b 100644 --- a/waiter/src/waiter/headers.clj +++ b/waiter/src/waiter/headers.clj @@ -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 diff --git a/waiter/src/waiter/process_request.clj b/waiter/src/waiter/process_request.clj index 4dd352b24..af92dfbba 100644 --- a/waiter/src/waiter/process_request.clj +++ b/waiter/src/waiter/process_request.clj @@ -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 @@ -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) diff --git a/waiter/src/waiter/websocket.clj b/waiter/src/waiter/websocket.clj index 51e33fc7d..851bc6588 100644 --- a/waiter/src/waiter/websocket.clj +++ b/waiter/src/waiter/websocket.clj @@ -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)) diff --git a/waiter/test/waiter/headers_test.clj b/waiter/test/waiter/headers_test.clj index e59e8eb49..0de272435 100644 --- a/waiter/test/waiter/headers_test.clj +++ b/waiter/test/waiter/headers_test.clj @@ -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))))) \ No newline at end of file + (dissoc-hop-by-hop-headers headers "HTTP/1.0"))))) \ No newline at end of file diff --git a/waiter/test/waiter/process_request_test.clj b/waiter/test/waiter/process_request_test.clj index 82fa2ba89..7e01a8f1c 100644 --- a/waiter/test/waiter/process_request_test.clj +++ b/waiter/test/waiter/process_request_test.clj @@ -315,9 +315,8 @@ 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))) @@ -325,25 +324,36 @@ (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" "test-user@test.com"})) (: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))