`setcookie()` expires_or_options param is integer not mixed

[xurizaemon]

See https://www.php.net/manual/en/function.setcookie.php - passing a boolean here as the third of >3 parameters triggers warning in output on PHP8.1 (which prevents cookie getting set and breaks cookie tests for me).

It's also valid post 7.3 to pass an array as the third parameter. From 8.1 passing other types (eg null in this instance) emits a deprecation notice, and this usage will fail from 9.0.

Steps to reproduce

To reproduce using minkphp/MinkGoutteDriver and PHP8.1

• git clone git@github.com:minkphp/MinkGoutteDriver
• cd MinkGoutteDriver
• docker run --rm -ti -v $(pwd):/code composer bash (currently this is PHP8.1)
• start ./vendor/bin/mink-test-server (I used a separate docker exec... bash on the same container to do this)
• ./vendor/bin/phpunit (output below)
• curl -s http://localhost:8002/sub-folder/cookie_page4.php | grep -E '(Deprecated|Warning)' to see why the test failed

bash-5.1# php -v
PHP 8.1.2 (cli) (built: Jan 21 2022 21:38:22) (NTS)
Copyright (c) The PHP Group

bash-5.1# ./vendor/bin/phpunit 
PHPUnit 9.5.13 by Sebastian Bergmann and contributors.

Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

.............................FF................................  63 / 118 ( 53%)
......S...S......S.....................................         118 / 118 (100%)

Time: 00:00.724, Memory: 14.00 MB

There were 2 failures:

1) Behat\Mink\Tests\Driver\Basic\CookieTest::testCookieInSubPath with data set #0 ('session_reset')
Failed asserting that 'Basic Form Previous cookie: NO' contains "Previous cookie: srv_var_is_set".

/code/vendor/mink/driver-testsuite/tests/Basic/CookieTest.php:113

2) Behat\Mink\Tests\Driver\Basic\CookieTest::testCookieInSubPath with data set #1 ('cookie_delete')
Failed asserting that 'Basic Form Previous cookie: NO' contains "Previous cookie: srv_var_is_set".

/code/vendor/mink/driver-testsuite/tests/Basic/CookieTest.php:113

FAILURES!
Tests: 118, Assertions: 331, Failures: 2, Skipped: 3.

bash-5.1# curl -s http://localhost:8002/sub-folder/cookie_page4.php | grep -E '(Deprecated|Warning)'
<b>Deprecated</b>:  setcookie(): Passing null to parameter #3 ($expires_or_options) of type array|int is deprecated in <b>/code/vendor/mink/driver-testsuite/web-fixtures/sub-folder/cookie_page4.php</b> on line <b>7</b><br />
<b>Warning</b>:  Cannot modify header information - headers already sent by (output started at /code/vendor/mink/driver-testsuite/web-fixtures/sub-folder/cookie_page4.php:7) in <b>/code/vendor/mink/driver-testsuite/web-fixtures/sub-folder/cookie_page4.php</b> on line <b>7</b><br />

[aik099]

@xurizaemon , Does it break driver tests on PHP 8.1 as well?

@stof , Do we actually test Mink on PHP 8.1?

[stof]

@aik099 this only triggers a deprecation warning, not a error. And so it will not break the testsuite on PHP 8.1 (it would break it for PHP 9.0). We do test Mink and the drivers on PHP 8.1. But this code is not executed as part of the testsuite (where we do report deprecations) but as part of the test server serving fixtures (where we don't report deprecations back to the testsuite)

[xurizaemon]

@aik099 it does for DMore/chrome-mink-driver - fail on 8.1, pass on 8.0 and earlier PHP versions via a WIP issue there to bring PHP support up to date. Since that driver's tests run with E_ALL the deprecation notice shows up in output and prevents the cookie working.

This change to swap null for 0 proposed here will make it consistent with the other setcookie() usages in this codebase (1 2 3). This null is a straggler IMO :)

To reproduce the deprecation notice in CLI:

$ docker run -ti php:8.1 php -a
Interactive shell

php > setcookie('foo', 'bar', null, '/');

Deprecated: setcookie(): Passing null to parameter #3 ($expires_or_options) of type array|int is deprecated in php shell code on line 1

It doesn't look like https://github.com/minkphp/driver-testsuite/blob/master/bin/mink-test-server sets error_reporting, so whether this notice will break cookie functionality on other drivers using this will depend on the error_reporting configuration that mink-test-server (etc) is run with (until 9.0 as @stof notes).

However, I see that MinkGoutteDriver sets error_reporting(-1) in "Setup PHP" and then does not fail eg here, which I'd expect to reveal the same behaviour. (I'm not familiar with that driver's tests yet!)

[xurizaemon]

Verified that this does fail the MinkGoutteDriver test suite when run locally.

See updated issue description for steps to reproduce using that driver.

[xurizaemon]

Hi! This PR addresses a PHP8 deprecation / PHP9 fatal error in test suite fixtures.

It was approved in late January. Issue description contains steps to reproduce and reference to PHP core details.

This error will break tests on PHP9. Until then it can be masked by error_reporting(-1) but I believe we should be proactive and fix it sooner not later.

What more can I do to assist in having this small correction merged?

[aik099]

Merging, thanks @xurizaemon .