-
Notifications
You must be signed in to change notification settings - Fork 53
RFC6265 compliant date parsing. #46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
[RFC6265 section 5.1.1](https://tools.ietf.org/html/rfc6265#section-5.1.1) describes algorithm which should be used to parse a cookie-date. Ported from here: https://github.com/salesforce/tough-cookie/blob/master/lib/cookie.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome @mekras, thank you.
is the cookie date format different from other date format specifications? i am unhappy about adding such a parser because of future maintainance - but if its a cookie specific thing, it does belong here. we could make the date parser its own static public method in a separate class. then it would also be easier to test all kind of edge cases. we should test it quite a bit to avoid it working worse in the end than php DateTime. |
Hm, separating logic makes sense. |
👍 And this could go in php-http/message as it's agnostic from HttpClient, then this plugin could use the parser. |
Oh yes, I totally missed this is not in message |
urks. whyever they needed to do that. but if thats the case we should support it. @sagikazarmark do you think we still should split the parser out of the cookie plugin? i'd say it should... |
Definitely, the plugin itself should contain as little logic as possible. |
Ping @mekras |
I'm here. How can I help? |
As discussed above, this parsing logic may better fit into the message package where we have other cookie related stuff as well. Would be nice if you could separate it: main logic->message and only use the parser here. I guess there is not much sense in abstracting this logic, so I could even imagine a static class doing this stuff. WDYT? |
I have been convinced many times that my vision of architecture is at variance with yours. So make your own decision. Just tell me what to do with this code, and I'll do it. |
@mekras Can you create the PR on https://github.com/php-http/message ? |
What's in this PR?
Implementation of a cookie-date parsing algorithm described in RFC6265 section 5.1.1. Ported from here: https://github.com/salesforce/tough-cookie/blob/master/lib/cookie.js
Why?
Using only DateTime::COOKIE format is not compliant with this RFC6265. For example valid date "Tuesday, 31 Mar 99 07:42:12 GMT" will cause an exception.
Checklist
To Do