couple of observations: 1 aesthetic, not sure why you changed cors_headers which was based on pattern to 2 functions but wtfm ;) We are always returning a content-type in couchdb maybe we could simplify the code here. I am not happy to modify the header case in the ExposedHeaders, maybe we coulld have one list transformed in a proplist {"lower", {"RealCase"} and test the other against it and eventually add the missing one using a lists:foldl. I won't have any time to make the patch tonight, these are only observations I am happy to discuss. - benoît On Tue, Dec 4, 2012 at 9:43 PM, wrote: > Updated Branches: > refs/heads/1368-fix-multipart-header-parts a67308e32 -> ce55c6e18 > (forced update) > > > handle exposed headers > > > Project: http://git-wip-us.apache.org/repos/asf/couchdb/repo > Commit: http://git-wip-us.apache.org/repos/asf/couchdb/commit/ce55c6e1 > Tree: http://git-wip-us.apache.org/repos/asf/couchdb/tree/ce55c6e1 > Diff: http://git-wip-us.apache.org/repos/asf/couchdb/diff/ce55c6e1 > > Branch: refs/heads/1368-fix-multipart-header-parts > Commit: ce55c6e18866411a4325b9caecab0af205ac07c2 > Parents: c432156 > Author: Jan Lehnardt > Authored: Mon Nov 12 13:53:04 2012 +0100 > Committer: Jan Lehnardt > Committed: Tue Dec 4 21:42:26 2012 +0100 > > ---------------------------------------------------------------------- > src/couchdb/couch_httpd.erl | 29 ++++++++++--------- > src/couchdb/couch_httpd_cors.erl | 50 +++++++++++++++++++++++++++++--- > test/etap/231-cors.t | 7 +++- > 3 files changed, 65 insertions(+), 21 deletions(-) > ---------------------------------------------------------------------- > > > > http://git-wip-us.apache.org/repos/asf/couchdb/blob/ce55c6e1/src/couchdb/couch_httpd.erl > ---------------------------------------------------------------------- > diff --git a/src/couchdb/couch_httpd.erl b/src/couchdb/couch_httpd.erl > index eea530c..b71a6fc 100644 > --- a/src/couchdb/couch_httpd.erl > +++ b/src/couchdb/couch_httpd.erl > @@ -462,10 +462,11 @@ serve_file(Req, RelativePath, DocumentRoot) -> > serve_file(#httpd{mochi_req=MochiReq}=Req, RelativePath, DocumentRoot, > ExtraHeaders) -> > log_request(Req, 200), > - {ok, MochiReq:serve_file(RelativePath, DocumentRoot, server_header() > ++ > - couch_httpd_cors:cors_headers(Req) ++ > - couch_httpd_auth:cookie_auth_header(Req, []) > ++ > - ExtraHeaders)}. > + ResponseHeaders = server_header() > + ++ couch_httpd_auth:cookie_auth_header(Req, []) > + ++ ExtraHeaders, > + {ok, MochiReq:serve_file(RelativePath, DocumentRoot, > + couch_httpd_cors:cors_headers(Req, ResponseHeaders))}. > > qs_value(Req, Key) -> > qs_value(Req, Key, undefined). > @@ -616,9 +617,9 @@ start_response_length(#httpd{mochi_req=MochiReq}=Req, > Code, Headers, Length) -> > log_request(Req, Code), > couch_stats_collector:increment({httpd_status_codes, Code}), > Headers1 = Headers ++ server_header() ++ > - couch_httpd_auth:cookie_auth_header(Req, Headers) ++ > - couch_httpd_cors:cors_headers(Req), > - Resp = MochiReq:start_response_length({Code, Headers1, Length}), > + couch_httpd_auth:cookie_auth_header(Req, Headers), > + Headers2 = couch_httpd_cors:cors_headers(Req, Headers1), > + Resp = MochiReq:start_response_length({Code, Headers2, Length}), > case MochiReq:get(method) of > 'HEAD' -> throw({http_head_abort, Resp}); > _ -> ok > @@ -629,8 +630,8 @@ start_response(#httpd{mochi_req=MochiReq}=Req, Code, > Headers) -> > log_request(Req, Code), > couch_stats_collector:increment({httpd_status_codes, Code}), > CookieHeader = couch_httpd_auth:cookie_auth_header(Req, Headers), > - Headers2 = Headers ++ server_header() ++ CookieHeader ++ > - couch_httpd_cors:cors_headers(Req), > + Headers1 = Headers ++ server_header() ++ CookieHeader, > + Headers2 = couch_httpd_cors:cors_headers(Req, Headers1), > Resp = MochiReq:start_response({Code, Headers2}), > case MochiReq:get(method) of > 'HEAD' -> throw({http_head_abort, Resp}); > @@ -664,9 +665,9 @@ start_chunked_response(#httpd{mochi_req=MochiReq}=Req, > Code, Headers) -> > couch_stats_collector:increment({httpd_status_codes, Code}), > Headers1 = http_1_0_keep_alive(MochiReq, Headers), > Headers2 = Headers1 ++ server_header() ++ > - couch_httpd_auth:cookie_auth_header(Req, Headers1) ++ > - couch_httpd_cors:cors_headers(Req), > - Resp = MochiReq:respond({Code, Headers2, chunked}), > + couch_httpd_auth:cookie_auth_header(Req, Headers1), > + Headers3 = couch_httpd_cors:cors_headers(Req, Headers2), > + Resp = MochiReq:respond({Code, Headers3, chunked}), > case MochiReq:get(method) of > 'HEAD' -> throw({http_head_abort, Resp}); > _ -> ok > @@ -695,10 +696,10 @@ send_response(#httpd{mochi_req=MochiReq}=Req, Code, > Headers, Body) -> > true -> ok > end, > Headers2 = Headers1 ++ server_header() ++ > - couch_httpd_cors:cors_headers(Req) ++ > couch_httpd_auth:cookie_auth_header(Req, Headers1), > + Headers3 = couch_httpd_cors:cors_headers(Req, Headers2), > > - {ok, MochiReq:respond({Code, Headers2, Body})}. > + {ok, MochiReq:respond({Code, Headers3, Body})}. > > send_method_not_allowed(Req, Methods) -> > send_error(Req, 405, [{"Allow", Methods}], <<"method_not_allowed">>, > ?l2b("Only " ++ Methods ++ " allowed")). > > > http://git-wip-us.apache.org/repos/asf/couchdb/blob/ce55c6e1/src/couchdb/couch_httpd_cors.erl > ---------------------------------------------------------------------- > diff --git a/src/couchdb/couch_httpd_cors.erl > b/src/couchdb/couch_httpd_cors.erl > index 17f634a..7cfefc3 100644 > --- a/src/couchdb/couch_httpd_cors.erl > +++ b/src/couchdb/couch_httpd_cors.erl > @@ -20,7 +20,7 @@ > > -include("couch_db.hrl"). > > --export([is_preflight_request/1, cors_headers/1]). > +-export([is_preflight_request/1, cors_headers/2]). > > -define(SUPPORTED_HEADERS, "Accept, Accept-Language, Content-Type," ++ > "Expires, Last-Modified, Pragma, Origin, Content-Length," ++ > @@ -30,6 +30,12 @@ > -define(SUPPORTED_METHODS, "GET, HEAD, POST, PUT, DELETE," ++ > "TRACE, CONNECT, COPY, OPTIONS"). > > +% as defined in http://www.w3.org/TR/cors/#terminology > +-define(SIMPLE_HEADERS, ["Cache-Control", "Content-Language", > + "Content-Type", "Expires", "Last-Modified", "Pragma"]). > +-define(SIMPLE_CONTENT_TYPE_VALUES, ["application/x-www-form-urlencoded", > + "multipart/form-data", "text/plain"]). > + > % TODO: - pick a sane default > -define(CORS_DEFAULT_MAX_AGE, 12345). > > @@ -173,11 +179,12 @@ > send_preflight_response(#httpd{mochi_req=MochiReq}=Req, Headers) -> > > % cors_headers/1 > > -cors_headers(MochiReq) -> > +cors_headers(MochiReq, RequestHeaders) -> > EnableCors = enable_cors(), > - cors_headers(MochiReq, EnableCors). > + CorsHeaders = do_cors_headers(MochiReq, EnableCors), > + maybe_apply_cors_headers(CorsHeaders, RequestHeaders). > > -cors_headers(#httpd{mochi_req=MochiReq}, true) -> > +do_cors_headers(#httpd{mochi_req=MochiReq}, true) -> > Host = couch_httpd_vhost:host(MochiReq), > AcceptedOrigins = get_accepted_origins(Host), > case MochiReq:get_header_value("Origin") of > @@ -191,9 +198,42 @@ cors_headers(#httpd{mochi_req=MochiReq}, true) -> > handle_cors_headers(couch_util:to_list(Origin), > Host, AcceptedOrigins) > end; > -cors_headers(_MochiReq, false) -> > +do_cors_headers(_MochiReq, false) -> > []. > > +maybe_apply_cors_headers([], RequestHeaders) -> > + RequestHeaders; > +maybe_apply_cors_headers(CorsHeaders, RequestHeaders) -> > + % for each RequestHeader that isn't in SimpleHeaders, > + % (or Content-Type with SIMPLE_CONTENT_TYPE_VALUES) > + % append to Access-Control-Exposed-Headers > + % return: RequestHeaders ++ CorsHeaders ++ ACEH > + > + LowerRequestHeaders = [string:to_lower(K) || {K,_V} <- > RequestHeaders], > + LowerSimpleHeaders = [string:to_lower(H) || H <- ?SIMPLE_HEADERS], > + > + ExposedHeaders0 = LowerRequestHeaders -- LowerSimpleHeaders, > + > + % here we may have not moved Content-Type into ExposedHeaders, > + % now we need to check whether the Content-Type valus is > + % in ?SIMPLE_CONTENT_TYPE_VALUES and if it isn’t add Content- > + % Type to to ExposedHeaders > + ContentType = string:to_lower( > + proplists:get_value("Content-Type", RequestHeaders)), > + > + IncludeContentType = lists:member(ContentType, > ?SIMPLE_CONTENT_TYPE_VALUES), > + ExposedHeaders = case IncludeContentType of > + false -> > + ExposedHeaders0 ++ ["Content-Type"]; > + true -> > + ExposedHeaders0 > + end, > + > + CorsHeaders > + ++ RequestHeaders > + ++ [{"Access-Control-Exposed-Headers", > + string:join(ExposedHeaders, ", ")}]. > + > > handle_cors_headers(_Origin, _Host, []) -> > []; > > > http://git-wip-us.apache.org/repos/asf/couchdb/blob/ce55c6e1/test/etap/231-cors.t > ---------------------------------------------------------------------- > diff --git a/test/etap/231-cors.t b/test/etap/231-cors.t > index efc19a6..3e44e1c 100644 > --- a/test/etap/231-cors.t > +++ b/test/etap/231-cors.t > @@ -32,7 +32,7 @@ server() -> > main(_) -> > test_util:init_code_path(), > > - etap:plan(25), > + etap:plan(27), > case (catch test()) of > ok -> > etap:end_tests(); > @@ -203,7 +203,10 @@ test_db_request(VHost) -> > {ok, _, RespHeaders, _Body} -> > etap:is(proplists:get_value("Access-Control-Allow-Origin", > RespHeaders), > "http://example.com", > - "db Access-Control-Allow-Origin ok"); > + "db Access-Control-Allow-Origin ok"), > + etap:is(proplists:get_value("Access-Control-Exposed-Headers", > RespHeaders), > + "server, Content-Type", > + "db Access-Control-ExposedHeaders ok"); > _ -> > etap:is(false, true, "ibrowse failed") > end. > >