Richer options for cookie generation and setting

[nmk]

I couldn't find a way of creating session cookies, so I added a function for doing this.

[agrafix]

Thanks for the Patch! You're right, that functionality is lacking. I think the name setSessionCookie might be misleading though, because it has nothing to do with Spocks sessions. This would be a breaking change, but I think the the clearest way to handle this would be to refactor setCookie and setCookie' into one function setCookie taking a

data CookieValid
     = CookieValidUntil UTCTime
     | CookieValidFor DiffTime
     | CookieValidForSession

instead of a UTCTime / DiffTime. What do you think? Are you interested in implementing this?

[nmk]

I will be happy to have a look this week. I also find your proposal to be the clearest one. Even if it breaks the current API, the error messages should be clear enough and the necessary changes in user code easy to make.

[nmk]

I have refactored the cookie generation and added a number of options (most importantly setting the domain, HTTPOnly and secure keys).

[agrafix]

Awesome, thank you!! I will review your code so we can get it into master.

[agrafix]

Great, thank you :-)

[nmk]

I wanted to add the Haddocks as well. Didn't realise that GH would notify you of a push to master.

Oh, well, I will open a PR soon, then. One thing I think is still missing is escaping of the name, value and possibly path fragments. I am not clear on whether this should be done, and how, but I will look into it.

[agrafix]

Not sure if GH notified me or I was just checking and saw your commit. The code was good for master so I merged it :-) Looking forward to your next PR!

Concerting the escaping of cookies: The json java library will replace +, %, =, ; with the corresponding %hh hex values. I think that sounds like a reasonable approach? We'd also need to implement unescaping in the read-cookie part. I think if we document this well there won't be any suprises for the users.

[nmk]

Please have a look at https://github.com/nmk/Spock/commit/dea9e74d90f84aee3c42a424aedc8c5d4b471131

As far as I can see, url encoding and decoding cookie values is a safe bet. Both Python's http/cookie and Ruby's Rack seem to do it.

The commit introduces a new dependency on string-conversions though this is just a convenience and I would not mind (much) if you would prefer the conversions to be explicit and the dependency removed.

Having the values urlencoded which basically means ASCII values all around, I think the final representation of a generated cookie header value should be a ByteString. This would save the last conversion from the Text -> ByteString -> (urlEncode) -> ByteString -> Text pipeline. The final Text value is probably converted back to a ByteString anyway for passing to the WAI functions for setting a header, correct?.

[agrafix]

Thanks!

• Yes, I think url encoding is the way to go, too. As mentioned above we'll just need to document it well.
• The string-conversions package is convenient, but I would personally try to keep that dependency out, because it's not strictly needed and just increases the risk of breaking. (A comparable package (http://hackage.haskell.org/package/stringable) just got deprecated for example...)
• Yes, we go with ByteStrings. (https://github.com/agrafix/Spock/blob/master/src/Web/Spock/Internal/Wire.hs#L142-L148) I think we can introduce setRawHeader, setRawMultiHeader next to setHeader (like header and rawHeader) taking a CI ByteString -> ByteString -> ... and MultiHeader -> ByteString -> ... respectively. We should probably try to work with bytestrings as much as possible and convert to text only at the end of the "pipeline".

[nmk]

I have given it a go in https://github.com/nmk/Spock/commit/78c99199e770c46b051d1da793259324bca6931f.

[agrafix]

Looks good, I only have one comment and then we can merge it (with documentation).

[agrafix]

Any news here? @nmk

[nmk]

I changed the type of cs_domain as you suggested.

I still haven't gotten around to writing haddocks, but will try to get it done this week.

[nmk]

I have added some documentation for the exported types and functions. I also rebased my fork on the current master, so a merge would be conflict-free.

[agrafix]

Great, can you send a pull request please?

Am 17.09.2015 um 10:10 schrieb Nickolay Kolev notifications@github.com:

I have added some documentation for the exported types and functions. I also rebased my fork on the current master, so a merge would be conflict-free.

— Reply to this email directly or view it on GitHub.