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

Issue fork drupal-3446569

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

poker10 created an issue. See original summary.

poker10’s picture

Issue summary: View changes
Status: Active » Needs review
Related issues: +#2399657: Add session hijacking test cases for SA-CORE-2014-006

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.

joseph.olstad’s picture

Status: Needs review » Reviewed & tested by the community

100% green here for both MRs above 8025 and 8029

poker10’s picture

Issue tags: +Pending Drupal 7 commit

Just to note, the MR to review/commit is 8029.

The MR 8025 is a test-only MR demonstrating, that the test was not working correctly even before (see #4).

  • mcdruid committed 8f0cd688 on 7.x
    Issue #3446569 by poker10, joseph.olstad: [D7 PHP 8.3] Fix...
mcdruid’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Pending Drupal 7 commit

Great, thank you!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.