Handle multiple Set-Cookie headers in replicator session plugin #5066
Conversation
Previously, replicator auth session plugin crashed if additional cookie headers were added besides the default `AuthSession` one. Fix replicator session plugin to consider only `Set-Cookie` headers with 'AuthSession' set and ignore others. Co-Authored-By: Robert Newson <[email protected]> Fix: #5064
| -spec get_cookies(list()) -> [list()]. | ||
| get_cookies(Headers) -> | ||
| Headers1 = mochiweb_headers:make(Headers), | ||
| Headers2 = mochiweb_headers:to_list(Headers1), |
There was a problem hiding this comment.
why do we need these two steps? Headers is already usable, has been case-folded to lowercase by mochiweb.
There was a problem hiding this comment.
Headers is not coming from mochiweb but ibrowse in whatever case they came in. So we do the standard mochiweb "raw" processing, normalization, etc. but the context is all about being on the client side, even though we're using our sever-side mochiweb library.
We could probably do that ourselves but since the headers does some extra stuff like combine headers, trim whitespace it might be safer just to process all headers the same way.
| Headers1 = mochiweb_headers:make(Headers), | ||
| Headers2 = mochiweb_headers:to_list(Headers1), | ||
| Fun = fun({K, V}) -> | ||
| case string:equal(K, "Set-Cookie", true) of |
There was a problem hiding this comment.
likewise we don't need a case-insensitive check here if the input was already forced to lower.
There was a problem hiding this comment.
We do, because to_list will returns the case format of the first entry for the header it finds. So we can look up the header by "set-cookie" and return the value, but since "looking up" in this case doesn't seem to work, we get the whole list so we have to do some of the case-insensitive match ourselves.
mochiweb_headers:to_list(mochiweb_headers:make([{"sEt-cooKie", "foo=bar"}, {"SeT-cooKie", "a=b"}, {"set-cookIe", "d=e"}])).
[{"sEt-cooKie","foo=bar"},
{"sEt-cooKie","a=b"},
{"sEt-cooKie","d=e"}]There was a problem hiding this 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.
Previously, replicator auth session plugin crashed if additional cookie headers were added besides the default
AuthSessionone.Fix replicator session plugin to consider only
Set-Cookieheaders withAuthSessionset and ignore others.Co-Authored-By: Robert Newson
Fix: #5064