rnewson commented on code in PR #5066:
URL: https://github.com/apache/couchdb/pull/5066#discussion_r1610139654


##########
src/couch_replicator/src/couch_replicator_auth_session.erl:
##########
@@ -386,23 +386,50 @@ http_response({error, Error}, #state{session_url = Url, 
user = User}) ->
     {error, {session_request_failed, Url, User, Error}}.
 
 -spec parse_cookie(list()) -> {ok, age(), string()} | {error, term()}.
-parse_cookie(Headers0) ->
-    Headers = mochiweb_headers:make(Headers0),
-    case mochiweb_headers:get_value("Set-Cookie", Headers) of
-        undefined ->
+parse_cookie(Headers) ->
+    case get_cookies(Headers) of
+        [] ->
             {error, cookie_not_found};
-        CookieHeader ->
-            CookieKVs = mochiweb_cookies:parse_cookie(CookieHeader),
-            CaseInsKVs = mochiweb_headers:make(CookieKVs),
-            case mochiweb_headers:get_value("AuthSession", CaseInsKVs) of
-                undefined ->
-                    {error, cookie_format_invalid};
-                Cookie ->
-                    MaxAge = parse_max_age(CaseInsKVs),
-                    {ok, MaxAge, Cookie}
+        [_ | _] = Cookies ->
+            case get_auth_session_cookies_and_age(Cookies) of
+                [] -> {error, cookie_format_invalid};
+                [{Cookie, MaxAge} | _] -> {ok, MaxAge, Cookie}
             end
     end.
 
+% Return list of cookies from headers, each as a KV list.
+% For example:
+%  [
+%    [{"AuthSession", "foo"}, {"max-age", "1"}],
+%    [{"ApiKey", "Secret"}, {"HttpOnly", []}]
+%  ]
+%
+-spec get_cookies(list()) -> [list()].
+get_cookies(Headers) ->
+    Headers1 = mochiweb_headers:make(Headers),
+    Headers2 = mochiweb_headers:to_list(Headers1),
+    Fun = fun({K, V}) ->
+        case string:equal(K, "Set-Cookie", true) of

Review Comment:
   hrm, that's subtle then. I couldn't get mochiweb to mix things up for me, 
but I guess I wasn't changing the first header.



##########
src/couch_replicator/src/couch_replicator_auth_session.erl:
##########
@@ -386,23 +386,50 @@ http_response({error, Error}, #state{session_url = Url, 
user = User}) ->
     {error, {session_request_failed, Url, User, Error}}.
 
 -spec parse_cookie(list()) -> {ok, age(), string()} | {error, term()}.
-parse_cookie(Headers0) ->
-    Headers = mochiweb_headers:make(Headers0),
-    case mochiweb_headers:get_value("Set-Cookie", Headers) of
-        undefined ->
+parse_cookie(Headers) ->
+    case get_cookies(Headers) of
+        [] ->
             {error, cookie_not_found};
-        CookieHeader ->
-            CookieKVs = mochiweb_cookies:parse_cookie(CookieHeader),
-            CaseInsKVs = mochiweb_headers:make(CookieKVs),
-            case mochiweb_headers:get_value("AuthSession", CaseInsKVs) of
-                undefined ->
-                    {error, cookie_format_invalid};
-                Cookie ->
-                    MaxAge = parse_max_age(CaseInsKVs),
-                    {ok, MaxAge, Cookie}
+        [_ | _] = Cookies ->
+            case get_auth_session_cookies_and_age(Cookies) of
+                [] -> {error, cookie_format_invalid};
+                [{Cookie, MaxAge} | _] -> {ok, MaxAge, Cookie}
             end
     end.
 
+% Return list of cookies from headers, each as a KV list.
+% For example:
+%  [
+%    [{"AuthSession", "foo"}, {"max-age", "1"}],
+%    [{"ApiKey", "Secret"}, {"HttpOnly", []}]
+%  ]
+%
+-spec get_cookies(list()) -> [list()].
+get_cookies(Headers) ->
+    Headers1 = mochiweb_headers:make(Headers),
+    Headers2 = mochiweb_headers:to_list(Headers1),
+    Fun = fun({K, V}) ->
+        case string:equal(K, "Set-Cookie", true) of
+            true -> {true, mochiweb_cookies:parse_cookie(V)};
+            false -> false
+        end
+    end,
+    lists:filtermap(Fun, Headers2).
+
+% From a list of cookies, pick out only AuthSession cookies.
+% Return a list of {Cookie, MaxAge} tuples
+%
+-spec get_auth_session_cookies_and_age([list()]) -> [{string(), age()}].
+get_auth_session_cookies_and_age(Cookies) ->
+    Fun = fun(CookieKVs) ->
+        CaseInsKVs = mochiweb_headers:make(CookieKVs),
+        case mochiweb_headers:get_value("AuthSession", CaseInsKVs) of

Review Comment:
   ok, convinced.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to