[BrowserKit] ignore invalid cookies expires date format #21462

Merged
merged 1 commit into from Jan 30, 2017

Projects

None yet

5 participants

@xabbuh
Member
xabbuh commented Jan 30, 2017
Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #15656
License MIT
Doc PR
@xabbuh xabbuh ignore invalid cookies expires date format
f19788d
@stof
Member
stof commented Jan 30, 2017

This looks weird to me. The issue says to ignore the cookie, not to ignore only its date

@xabbuh
Member
xabbuh commented Jan 30, 2017

Not sure what you mean. The issue description contains this quote from the RFC:

If the attribute-value failed to parse as a cookie date, ignore the cookie-av.

Maybe I misinterpret that part, but IIUC cookie-av means cookie argument value, but the entire cookie.

@stof
Member
stof commented Jan 30, 2017

ah sorry, I misread it

@javiereguiluz

I guess we should also update the constructor of this class, where you can find:

if (null !== $expires) {
    $timestampAsDateTime = \DateTime::createFromFormat('U', $expires);
    if (false === $timestampAsDateTime) {
        throw new \UnexpectedValueException(sprintf('The cookie expiration time "%s" is not valid.', $expires));
    }

    $this->expires = $timestampAsDateTime->format('U');
}
@xabbuh
Member
xabbuh commented Jan 30, 2017

@javiereguiluz I don't think so. Let me quote myself from #15656 (comment):

Actually, after thinking of this again I do not think that we need to change the constructor. It is documented that the expected value must be a string containing the expires timestamp.

The constructor will be used by some code that already had to parse the Set-Cookie header. So this part should be responsible to deal with invalid values instead of magically treating them as anything else.

@javiereguiluz

@xabbuh you are right! I missed your comment.


👍

Status: reviewed

@fabpot
Member
fabpot commented Jan 30, 2017

Thank you @xabbuh.

@fabpot fabpot merged commit f19788d into symfony:2.7 Jan 30, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details
@fabpot fabpot added a commit that referenced this pull request Jan 30, 2017
@fabpot fabpot bug #21462 [BrowserKit] ignore invalid cookies expires date format (x…
…abbuh)

This PR was merged into the 2.7 branch.

Discussion
----------

[BrowserKit] ignore invalid cookies expires date format

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #15656
| License       | MIT
| Doc PR        |

Commits
-------

f19788d ignore invalid cookies expires date format
a6c05e5
@xabbuh xabbuh deleted the xabbuh:issue-15656 branch Jan 30, 2017
This was referenced Feb 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment