From 09c1a241f9c787f6029c81a646f51eb5b722b0dd Mon Sep 17 00:00:00 2001
From: Daniel White <daniel@whitehouse.id.au>
Date: Sat, 2 Nov 2013 23:11:07 +1100
Subject: [PATCH] Use a single header for Access-Control-Expose-Headers

This fixes the case where more than one header is exposed.
Previously, only one header would be listed due to the way cowboy
handles multiple response headers with the same name.

If we had more control over the reply, we could perhaps add multiple
headers (due to how cowboy_req:reply/4 works).  However, this approach
is much simpler and appears to be more consistent with its use in the
wild.
---
 src/cowboy_cors.erl | 15 ++++++++++++---
 test/cors_SUITE.erl | 17 +++++++++++++++++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/src/cowboy_cors.erl b/src/cowboy_cors.erl
index a68aef1..daada32 100644
--- a/src/cowboy_cors.erl
+++ b/src/cowboy_cors.erl
@@ -139,9 +139,9 @@ exposed_headers(Req, State) ->
 
 set_exposed_headers(Req, []) ->
     Req;
-set_exposed_headers(Req, [Header|Tail]) ->
-    Req1 = cowboy_req:set_resp_header(<<"access-control-expose-headers">>, Header, Req),
-    set_exposed_headers(Req1, Tail).
+set_exposed_headers(Req, Headers) ->
+    Bin = header_list(Headers),
+    cowboy_req:set_resp_header(<<"access-control-expose-headers">>, Bin, Req).
 
 %% allow_credentials/1 should return true or false.
 allow_credentials(Req, State) ->
@@ -195,3 +195,12 @@ terminate(Req, #state{env = Env}) ->
 -spec error_terminate(cowboy_req:req(), #state{}) -> no_return().
 error_terminate(_Req, _State) ->
     erlang:throw({?MODULE, error}).
+
+%% create a comma-separated list for a header value
+header_list(Values) ->
+    header_list(Values, <<>>).
+
+header_list([Value], Acc) ->
+    <<Acc/binary, Value/binary>>;
+header_list([Value | Rest], Acc) ->
+    header_list(Rest, <<Acc/binary, Value/binary, ",">>).
diff --git a/test/cors_SUITE.erl b/test/cors_SUITE.erl
index 22e0128..6bdf44f 100644
--- a/test/cors_SUITE.erl
+++ b/test/cors_SUITE.erl
@@ -17,6 +17,7 @@
 -export([standard_options/1]).
 -export([simple_allowed_get/1]).
 -export([simple_allowed_credentials_get/1]).
+-export([simple_exposed_headers/1]).
 -export([actual_options/1]).
 -export([preflight_method/1]).
 -export([preflight_allowed_method/1]).
@@ -46,6 +47,7 @@ groups() ->
                            standard_options,
                            simple_allowed_get,
                            simple_allowed_credentials_get,
+                           simple_exposed_headers,
                            actual_options,
                            preflight_method,
                            preflight_allowed_method,
@@ -168,6 +170,21 @@ simple_allowed_credentials_get(Config) ->
     {_, Origin} = lists:keyfind(<<"access-control-allow-origin">>, 1, Headers),
     {_, <<"true">>} = lists:keyfind(<<"access-control-allow-credentials">>, 1, Headers).
 
+simple_exposed_headers(Config) ->
+    Origin = <<"http://example.com">>,
+    Exposed = [<<"x-first">>, <<"x-second">>],
+    {ok, 204, Headers, _} =
+        request(<<"GET">>,
+                [{<<"Origin">>, Origin}],
+                [{allowed_origins, Origin},
+                 {allowed_methods, <<"GET">>},
+                 {exposed_headers, Exposed}],
+                Config),
+    {_, Origin} = lists:keyfind(<<"access-control-allow-origin">>, 1, Headers),
+    {_, ExposedList} = lists:keyfind(<<"access-control-expose-headers">>, 1, Headers),
+    Exposed = cowboy_http:nonempty_list(ExposedList, fun cowboy_http:token/2),
+    false = lists:keyfind(<<"access-control-allow-credentials">>, 1, Headers).
+
 actual_options(Config) ->
     %% OPTIONS request without Access-Control-Request-Method is not a pre-flight request.
     Origin = <<"http://example.com">>,