[D7 PHP 8.3] Fix SessionHttpsTestCase->testEmptySessionId() failure

Open on Drupal.org →

Created on 10 May 2024, 7 months ago

Updated 17 September 2024, 2 months ago

Problem/Motivation

On PHP 8.3, there is a failure in SessionHttpsTestCase->testEmptySessionId(), see:

---- SessionHttpsTestCase ----
Status    Group      Filename          Line Function                            
--------------------------------------------------------------------------------
Fail      Other      session.test       786 SessionHttpsTestCase->testEmptySessionId()
    The user's own name is not displayed because the invalid session cookie has
    logged them out.

We need to fix the test failure to have full PHP 8.3 compatibility.

Steps to reproduce

See some PHP 8.3 test job on GitlabCI: https://git.drupalcode.org/project/drupal/-/jobs/1568219

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

🐛 Bug report

Status

Version

7.0 ⚰️

Component

Last updated about 2 months ago

Maintained by
🇬🇧United Kingdom @alexpott
🇨🇭Switzerland @berdir
🇩🇪Germany @dawehner
🇮🇹Italy @mondrake

Created by

🇸🇰Slovakia poker10

Live updates comments and jobs are added and updated live.

PHP 8.3

Sign in to follow issues

Merge Requests

!8029[D7 PHP 8.3] Fix SessionHttpsTestCase->testEmptySessionId() failure
Open
🇸🇰Slovakia poker10
updated 7 months ago
!8025[D7 PHP 8.3] Fix SessionHttpsTestCase->testEmptySessionId() failure
Open
🇸🇰Slovakia poker10
updated 7 months ago

Comments & Activities

Issue created by @poker10
Merge request !8025Draft: Test the SessionHttpsTestCase->testEmptySessionId() test → (Open) created by poker10
Merge request !8029Issue #3446569: [D7 PHP 8.3] Fix SessionHttpsTestCase->testEmptySessionId() failure → (Open) created by poker10
Open in Jenkins → Open on Drupal.org →
Environment: PHP 8.1 & MySQL 5.7
last update 7 months ago
2,179 pass
Open in Jenkins → Open on Drupal.org →
Environment: php8.3_sqlite3.34
last update 7 months ago
run-tests.sh exception
Pipeline finished with Success
7 months ago
Total: 524s
#170155
Open in Jenkins → Open on Drupal.org →
Environment: php8.3_sqlite3.34
last update 7 months ago
2,174 pass, 1 fail
Status changed to Needs review 7 months ago10:22pm 10 May 2024
Comment 7 months ago →
🇸🇰Slovakia poker10
I tried to reproduce this failure on multiple enviroments: local (linux, windows), our demo server, .. and was unable to reproduce it anywhere, so was unable to debug it. I think it could probably be related to some PHP enviroment config.

While trying to debug the SessionHttpsTestCase->testEmptySessionId() test, I found out, that the test seems not working at all.

I have created a test branch with a test MR: https://git.drupalcode.org/project/drupal/-/merge_requests/8025 . I have reverted the SA-CORE-2014-006 security fix in session.inc there. Then I expected that the SessionHttpsTestCase->testEmptySessionId() test would start failing. And it does not, it is still green, see: https://git.drupalcode.org/project/drupal/-/pipelines/170033

Therefore I think the test is currently not working and it is questionable, if it was working correctly in the past.

----------

So I think that the one solution is to remove the non-working test (not created a MR yet). Other possible solution is to use something similar as it is used in SessionTestCase::testEmptySessionID() - add a $this->curlClose(); just before the drupalGet() call. This should at least fix the failure on PHP 8.3.

I have created a MR 8029 for the second possible solution here: https://git.drupalcode.org/project/drupal/-/merge_requests/8029

All current tests seems to be green with this change: https://git.drupalcode.org/project/drupal/-/pipelines/170155 . Also the PHP 8.3 test is without this failure (only the HTML parsing failure from TextSummaryTestCase() is still present after applying the fix from here), see: https://www.drupal.org/pift-ci-job/2893116 → .

For illustration, if we combine fix from this issue and fix from the other one, the 8.3 testing is green: https://git.drupalcode.org/project/drupal/-/pipelines/170105 (see the META issue).

Moving to Needs review.
Status changed to RTBC 2 months ago2:53pm 17 September 2024
Comment 2 months ago →
🇨🇦Canada joseph.olstad
100% green here for both MRs above 8025 and 8029

contrib.social Blog FAQ Discussions

Production build 0.71.5 2024