php-http / message

HTTP Message related tools
http://php-http.org
MIT License
1.29k stars 41 forks source link

Add cookie #22

Closed sagikazarmark closed 8 years ago

sagikazarmark commented 8 years ago

This PR adds Cookie implementation as discussed in php-http/httplug#98

Fix #15

A few open issues left in the original repository: https://github.com/php-http/cookie/issues

sagikazarmark commented 8 years ago

Thanks for the feedback.

sagikazarmark commented 8 years ago

About the expires thing: currently you can pass an expiration parameter in the constructor. Should we have max age instead. Also, this expiration is set for both max age and expires property ATM.

Should we keep both max age and expires and only remove the magic expiration method?

dbu commented 8 years ago

i would allow to set both from the constructor, to make it possible to model a response precisely.

but remove the magic method.

and phpdoc that expire is http 1.0 and should be avoided.

sagikazarmark commented 8 years ago

What about order of arguments: should expires be the last?

dbu commented 8 years ago

:+1: agree to have expires last so it can be ignored when not recording an exact Server response

sagikazarmark commented 8 years ago

We could add cookie related helpers here.

One thing I am thinking about:

We could have the cookie parser as is OR provide a compat package around this method:

http://php.net/manual/en/function.http-parse-cookie.php

In general, we could have a pecl_http compatibility layer, but I have never used it before and don't plan in the future, so I wonder if it makes sense.

sagikazarmark commented 8 years ago

Let's make cookie parsers in another PR. I think this is good to merge. Checked RFC compliance, but I am not sure about it. I've read the RFC several times, it is not always clear. From what I can understand, everything is OK.

dbu commented 8 years ago

the pecl_http compatibility layer sounds like a good idea but lets move forward. it can always be added later.