Problem/Motivation

Discovered in #3277649: Deprecation notice for htmlspecialchars with php 8.1, which calls file_requirements() from a kernel test.

Steps to reproduce

Proposed resolution

Add a ?? '' after \Drupal::request()->server->get('SERVER_SOFTWARE')

Remaining tasks

Add an inline comment explaining why this is necessary

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3279289

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:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir created an issue. See original summary.

cilefen’s picture

cilefen CreditAttribution: cilefen commented

@Berdir:

I think that rather than the null coalesce operator the second parameter to \Drupal::request()->server->get() should be set to an empty string. Agree?

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

That's fine too, although I have just recently come across a really weird case in the Symfony Request class where a key was set to NULL explicitly and then the fallback didn't work because Symfony uses arary_key_exists() for the fallback and explicitly "supports" NULL elements.

rizzie made their first commit to this issue’s fork.

rizzie opened merge request !2247

rizzie’s picture

rizzie CreditAttribution: rizzie commented
Status: Active » Needs review
cilefen’s picture

cilefen CreditAttribution: cilefen commented

I am adding myself a mentor credit for walking @rizzie through this.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon for European Commission and European Union Institutions, Agencies and Bodies commented
Version: 9.5.x-dev » 9.4.x-dev
Status: Needs review » Reviewed & tested by the community

I hit this with 8.1. Tested and works as it should, thanks. I think it's a bug and could be merged in 9.4.x.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Needs work

Can we get an inline comment explaining why this is necessary? Otherwise it might get "cleaned up" in the future. :) Thanks!

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Issue tags: +Prague2022
ChrisDarke’s picture

ChrisDarke CreditAttribution: ChrisDarke commented

This issue is tagged for first time contributors at DrupalCon Prague 2022.

ChrisDarke’s picture

ChrisDarke CreditAttribution: ChrisDarke commented
Issue summary: View changes
floWweb’s picture

floWweb CreditAttribution: floWweb at WebstanZ commented

hi from drupalcon2022! It's my firt time contributor and i want to test contribute for this issue

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

@floWweb: Great. This is a good first time task to actually make the change, but maybe coming up with the description not quite as much. I would say that it should mention that server software might not be set in a CLI environment and we ensure that it is a string for the regular expression check below.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon for European Commission and European Union Institutions, Agencies and Bodies commented
Status: Needs work » Reviewed & tested by the community

  • catch committed f5766a4 on 10.0.x 
    Issue #3279289 by rizzie, claudiu.cristea, Berdir, cilefen:...
  • catch committed 784ee65 on 10.1.x
    Issue #3279289 by rizzie, claudiu.cristea, Berdir, cilefen:...
  • catch committed 64581a5 on 9.5.x
    Issue #3279289 by rizzie, claudiu.cristea, Berdir, cilefen:...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 9.4.x-dev » 9.5.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.1.x, cherry-picked to 10.0.x and 9.5.x, thanks!

Status: Fixed » Closed (fixed)

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