Permalink
Browse files

Fix parse_set_cookie/1 and format_set_cookie/1 functions

1. According to the RFCs 2109 and 2965, multiple cookies can be set in a
single 'Set-Cookie' header. So, yaws_api:parse_set_cookie/1 now returns a
list of #setcookie{} records. If no cookie was found or if an error occurred,
it returns []. The parsing is also improved.
Note that this fix breaks the compatibility with previous versions.

2. In yaws_api:format_set_cookie/1, options are now always formated as
quoted-strings.

3. 2 new functions are added, yaws_api:parse_cookie/1 and
yaws_api:format_cookie/1, to parse and format 'Cookie' headers. To let these
functions to work, the #cookie{} record was introduced.

Documentation and testsuite are updated accordingly.
  • Loading branch information...
1 parent be0babd commit 3a8e0710c569f7e5c2702ab807fa0a994d221ad3 @capflam capflam committed Jul 4, 2012
Showing with 636 additions and 161 deletions.
  1. +13 −3 include/yaws_api.hrl
  2. +62 −2 man/yaws_api.5
  3. +290 −156 src/yaws_api.erl
  4. +271 −0 test/eunit/cookies.erl
View
@@ -90,19 +90,29 @@
-record(setcookie,{
key,
value,
- quoted,
+ quoted = false,
comment,
comment_url,
- discard,
+ discard = false,
domain,
max_age,
expires,
path,
port,
- secure,
+ secure = false,
version}).
+-record(cookie,{
+ key,
+ value,
+ quoted = false,
+ version = "0",
+ domain,
+ path,
+ port}).
+
+
-record(redir_self, {
host, % string() - our own host
scheme, % http | https
View
@@ -131,16 +131,76 @@ Sets a cookie to the browser.
\fBfind_cookie_val(Cookie, Header)\fR
This function can be used to search for a cookie that was previously
set by \fBsetcookie/2-6\fR. For example if we set a cookie
-as \fByaws_api:setcookie("sid",SomeRandomSid) \fR, then on subsequent requests
+as \fByaws_api:setcookie("sid",SomeRandomSid)\fR, then on subsequent requests
from the browser we can call:
\fBfind_cookie("sid",(Arg#arg.headers)#headers.cookie)\fR
The function returns [] if no cookie was found, otherwise the actual cookie
is returned as a string.
+.TP
+\fBparse_set_cookie(Str)\fR
+This function parses the value of a \fBSet-Cookie\fR header. Because multiple
+cookies can be set in a single \fBSet-Cookie\fR header, this function returns a
+list of \fI#setcookie{}\fR records. If no cookie was found or if an error
+occurred, it returns [].
+
+\fI#setcookie{}\fR record is defined in \fIyaws_api.hrl\fR:
+\fI
+.nf
+
+-record(setcookie, {
+ key,
+ value,
+ quoted = false,
+ comment,
+ comment_url,
+ discard = false,
+ domain,
+ max_age,
+ expires,
+ path,
+ port,
+ secure = false,
+ version
+ }).
+.fi
+\fR
+
+.TP
+\fBparse_cookie(Str)\fR
+This function does the same thing than \fBparse_set_cookie/1\fR but for the
+value of a \fBCookie\fR header. It returns a list of \fI#cookie{}\fR records. If
+no cookie was found or if an error occurred, it returns [].
+
+\fI#cookie{}\fR record is defined in \fIyaws_api.hrl\fR:
+\fI
+.nf
+
+-record(cookie, {
+ key,
+ value,
+ quoted = false,
+ version = "0",
+ domain,
+ path,
+ port}).
+ }).
+.fi
+\fR
+
+.TP
+\fBformat_set_cookie(Str)\fR
+Build a cookie string from a \fI#setcookie{}\fR record like returned by
+\fBparse_set_cookie/1\fR.
+
+.TP
+\fBformat_cookie(Str)\fR
+Build a cookie string from a \fI#cookie{}\fR record like returned by
+\fBparse_cookie/1\fR.
.TP
-\fBredirect(Url\fR
+\fBredirect(Url)\fR
This function generates a redirect to the browser.
It will clear any previously set headers. So to generate
a redirect \fBand\fR set a cookie, we need to set the cookie after
Oops, something went wrong.

3 comments on commit 3a8e071

It seems commits like that change a lot of code. How can we ensure the quality of such large changes? I know there is unit test added. But maybe a more thorough test and code review would be necessary to guard any performance degradation and compatibility break before it is commited into main branch.
Also a suggestion: yaws_api:find_cookie_val accepts either Arg or a list as the second argument. But we can change them into two functions so that we don't need to get to pattern matching the first one during each tail recursion for a list. like adding a function yaws_api:find_cookie_val_from_list(Name,List).

Collaborator

capflam replied Jul 5, 2012

First, about yaws_api:find_cookie_val/2, keeping API unchanged has advantage to not break backward compatibility. But, you're right, it's useless and inefficient to recurse on this function. find_cookie_val2 is a better choice for that. So, I will fix that quickly (I also found a small inconsistency in this function...).

Then, about this commit, changes are not so huge. The only thing that really changed is the function yaws_api:parse_set_cookie/1 that was buggy (according to the rfc2109 and rfc2965). yaws_api:find_cookie_val/2 will be slower and can be certainly improved. But, because the cookie parsing was improved, this function is less buggy (I hope so :). I've thought that changes was small enough to be merged in the master.

Steve and I should work again on this part to deal with rfc6265. So, discussions are always opened and all feedback are welcomed. Thanks for your comments.

Thanks for the quick response. That clears the things up.

I also opened another issue about the commit that adds an extra parameter IPPort to 'GET','POST' functions etc. Would you like to look into it as well? Thank you

Please sign in to comment.