Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Check that the content-type from the RequestHeaders is not undefined.…

… Fixes COUCHDB-1697
  • Loading branch information...
commit bc3ef5b8cfd58b9d07868c1267c4099f994e53d8 1 parent 0235722
Ryan Ramage ryanramage authored
Showing with 8 additions and 4 deletions.
  1. +8 −4 src/couchdb/couch_httpd_cors.erl
12 src/couchdb/couch_httpd_cors.erl
View
@@ -216,10 +216,14 @@ maybe_apply_cors_headers(CorsHeaders, RequestHeaders0) ->
% 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", RequestHeaders0)),
-
- IncludeContentType = lists:member(ContentType, ?SIMPLE_CONTENT_TYPE_VALUES),
+ ContentType = proplists:get_value("Content-Type", RequestHeaders0),
Dave Cottlehuber
dch added a note

How about ditching the case completely and use:

lists:member(string:to_lower(ContentType), ?SIMPLE_CONTENT_TYPE_VALUES).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ IncludeContentType = case ContentType of
+ undefined ->
+ false;
+ _ ->
+ ContentType_ = string:to_lower(ContentType),
+ lists:member(ContentType_, ?SIMPLE_CONTENT_TYPE_VALUES)
+ end,
ExposedHeaders = case IncludeContentType of
false ->
lists:umerge(ExposedHeaders0, ["Content-Type"]);

2 comments on commit bc3ef5b

Dave Cottlehuber

How about ditching the case completely and use:

lists:member(string:to_lower(ContentType), ?SIMPLE_CONTENT_TYPE_VALUES).

Dave Cottlehuber

LGTM, other than use of _ above. Not sure if that's kosher or not.

A possible patch on your patch:

diff --git i/src/couchdb/couch_httpd_cors.erl w/src/couchdb/couch_httpd_cors.erl
index ce41557..d369dc9 100644
--- i/src/couchdb/couch_httpd_cors.erl
+++ w/src/couchdb/couch_httpd_cors.erl
@@ -217,25 +217,19 @@ maybe_apply_cors_headers(CorsHeaders, RequestHeaders0) ->
     % in ?SIMPLE_CONTENT_TYPE_VALUES and if it isn’t add Content-
     % Type to to ExposedHeaders
     ContentType =  proplists:get_value("Content-Type", RequestHeaders0),
-    IncludeContentType = case ContentType of
-    undefined ->
-        false;
-    _ ->
-        ContentType_ = string:to_lower(ContentType),
-        lists:member(ContentType_, ?SIMPLE_CONTENT_TYPE_VALUES)
-    end,
-    ExposedHeaders = case IncludeContentType of
-    false ->
-        lists:umerge(ExposedHeaders0, ["Content-Type"]);
+    Maybe_add_ContentType =
+        lists:member(string:to_lower(ContentType), ?SIMPLE_CONTENT_TYPE_VALUES),
+    ExposedHeaders = case Maybe_add_ContentType of
     true ->
-        ExposedHeaders0
+        ExposedHeaders0;
+    _ ->
+        lists:umerge(ExposedHeaders0, ["Content-Type"])
     end,
     CorsHeaders
     ++ RequestHeaders0
     ++ [{"Access-Control-Expose-Headers",
             string:join(ExposedHeaders, ", ")}].

-
 reduce_headers(A, B) ->
     reduce_headers0(A, B, []).
Ryan Ramage

The problem is that ContentType comes back as undefined occasionally, so having

lists:member(string:to_lower(ContentType), ?SIMPLE_CONTENT_TYPE_VALUES),

will fail with the same error

[info] [<0.148.0>] Stacktrace: [{string,to_lower,
[undefined],
[
{file,"string.erl"}

So ContentType has to be tested for undefined before proceeding...maybe I am missing something, or moar suggestions welcome

Please sign in to comment.
Something went wrong with that request. Please try again.