Skip to content

Commit

Permalink
Simplify Whitelisted Headers
Browse files Browse the repository at this point in the history
Refactor simple-headers-wo-content-type to access-control-request-headers-whitelist.

The whitelist behaves differently by `set/difference`-ing
`access-control-request-headers` instead of `set/union` the
`access-control-allowed-headers`. This way, we do not put unnecessary
headers in the response `access-control-allowed-headers`.

From testing, the only required header in the whitelist is
"Origin". Safari always sends this in "Access-Control-Request-Headers"
during pre-flight, whereas the Firefox and Chrome do not.
  • Loading branch information
Raymond Huang committed Mar 5, 2016
1 parent caef1d4 commit 57c0426
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 15 deletions.
4 changes: 2 additions & 2 deletions src/com/unbounce/encors/core.clj
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@
;; Request -> CorsPolicy -> [:left [ErrorMsg]] | [:right Headers]
(defn cors-preflight-check-request-headers
[{:keys [headers] :as req} cors-policy]
(let [supported-headers (set/union
(let [supported-headers (set/difference
(:request-headers cors-policy)
types/simple-headers-wo-content-type)
types/access-control-request-headers-whitelist)
supported-headers* (into
(sorted-set)
(map str/lower-case supported-headers))
Expand Down
6 changes: 3 additions & 3 deletions src/com/unbounce/encors/types.clj
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@

(def simple-methods #{:get :head :post})

(def simple-headers #{"Accept" "Accept-Language" "Content-Language" "Content-Type" "Origin"})

(def simple-headers-wo-content-type #{"Accept" "Accept-Language" "Content-Language" "Origin"})
(def access-control-request-headers-whitelist
#{"Origin" ;Only Safari puts this in Access-Contol-Request-Headers.
})
13 changes: 3 additions & 10 deletions test/com/unbounce/encors/core_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -93,17 +93,13 @@
[:left [(str "HTTP headers requested in Access-Control-Request-Headers of "
"CORS request is not supported; requested: "
"'X-Not-Safe-To-Expose, X-Blah-Bleh'; "
"supported are 'X-Safe-To-Expose, Origin, Accept-Language, "
"Content-Language, Accept'.")]])))
"supported are 'X-Safe-To-Expose'.")]])))
(testing "Access-Control-Request-Headers match policy request headers"
(is (= (cors-preflight-check-request-headers
{:headers {"access-control-request-headers"
"X-Safe-To-Expose"}}
policy)
[:right {"Access-Control-Allow-Headers"
(str/join ", "
(set/union #{"X-Safe-To-Expose"}
types/simple-headers-wo-content-type))}])))))
[:right {"Access-Control-Allow-Headers" "X-Safe-To-Expose"}])))))

(deftest cors-preflight-headers-test
(testing "With a request that complies with policy"
Expand All @@ -117,10 +113,7 @@
"access-control-request-method"
"GET"}}
policy)
[:right {"Access-Control-Allow-Headers"
(str/join ", "
(set/union #{"X-Cool-Header"}
types/simple-headers-wo-content-type))
[:right {"Access-Control-Allow-Headers" "X-Cool-Header"
"Access-Control-Allow-Methods" "GET, HEAD, POST"
"Access-Control-Max-Age" "365"}])))))

Expand Down

0 comments on commit 57c0426

Please sign in to comment.