Skip to content

Fix cookie domain match #5

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

Merged
merged 2 commits into from
Feb 15, 2020
Merged

Fix cookie domain match #5

merged 2 commits into from
Feb 15, 2020

Conversation

Josh-G
Copy link
Contributor

@Josh-G Josh-G commented Feb 7, 2020

php-webdriver changed the signature of WebDriverOptions::getCookies in v1.4.0. They attempted to keep it backwards-compatible, however it did result in some BC break.

WebDriver::cookieDomainMatchesConfigUrl continues to treat these values as arrays, however they are now objects that implement ArrayAccess. When calling:

array_key_exists('domain', $cookie)

this returns false as it is not ArrayAccess aware.

This means that using any version of php-webdriver greater than 1.3.0 will result in the rest of the function becoming unreachable.

Another serious problem is that 7.4 now raises a deprecation notice when calling array_key_exists on a non-array (and can be raised as an exception).

In the test case, I have modified it to create Cookie objects instead of arrays. Codeception/Codeception#5480 reverted this, however the version constraint is now sufficient, ^1.6.0.

In the method, I have opted to use isset instead, as the class convention is to continue treating it as an array, although this is deprecated.

calling array_key_exists on classes implementing \ArrayAccess always 
returns false and emits a deprecation notice on 7.4
@Josh-G
Copy link
Contributor Author

Josh-G commented Feb 7, 2020

ci fail seems unrelated to me

@Naktibalda
Copy link
Member

Thanks

@Naktibalda Naktibalda merged commit aa7c2d6 into Codeception:master Feb 15, 2020
@Naktibalda
Copy link
Member

Released as 1.0.3

@Josh-G Josh-G deleted the fix-cookie-domain-match branch February 17, 2020 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants