rnewson commented on code in PR #5066:
URL: https://github.com/apache/couchdb/pull/5066#discussion_r1609381337
##########
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:
it is safer to use `CookieKVs = mochiweb_cookies:parse_cookie(Cookie)` which
creates a proplist (though note that mochiweb appears to have a bug when
parsing the `Expires` attribute. fortunately we don't need to examine it). once
its a proplist we can extract properties with
`proplists:get_value("AuthSession", CookieKVs) ` and
`list_to_integer(proplists:get_value("Max-Age", CookieKVs)`, etc.
##########
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),
Review Comment:
why do we need these two steps? `Headers` is already usable, has been
case-folded to lowercase by mochiweb.
##########
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:
likewise we don't need a case-insensitive check here if the input was
already forced to lower.
--
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]
