Problem/Motivation

Skip ReverseProxyMiddlewareTest::testReverseProxyEnabledLegacy test if Request::setTrustedHeaderName() does not exist because it's a legacy test and this doesn't exist in Symfony 4.

Proposed resolution

See problem motivation

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

N/a

CommentFileSizeAuthor
#4 3038062-3.patch2.07 KBGábor Hojtsy
#4 3038062-3-with-2976394-144.patch174.13 KBGábor Hojtsy
#3 3038062-3.patch2.07 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Issue tags: +Symfony 4, +Drupal 9
Related issues: +#3030501: [Symfony 4] Drupal\Core\StackMiddleware\ReverseProxyMiddleware calls Symfony\Component\HttpFoundation\Request::setTrustedHeaderName() which does not exist
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Active » Needs review
FileSize
3038062-3.patch2.07 KB

This removes the mention and testing of setTrustedHeaderName for no legacy test code.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
FileSize
3038062-3-with-2976394-144.patch174.13 KB
3038062-3.patch2.07 KB

Rolled together with #2976394-144: Allow Symfony 4.4 to be installed in Drupal 8 for testing purposes. Also uploading the prior patch unchanged again for easier commit :)

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Parent issue: » #2976394: Allow Symfony 4.4 to be installed in Drupal 8

The last submitted patch, 4: 3038062-3-with-2976394-144.patch, failed testing. View results

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Needs review » Reviewed & tested by the community

Ok it does not only look good but also actually fixes the issue :)

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed b4215ff and pushed to 8.7.x. Thanks!

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
CreditAttribution: kim.pepper at PreviousNext commented
Status: Fixed » Needs work

This doesn't look like the commit was pushed?

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs work » Reviewed & tested by the community

proper status

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Reviewed & tested by the community » Fixed

@catch now pushed the commit to 8.7 and I cherry-picked to 8.8, so all is good :) Thanks for noticing!

  • catch committed 78b4e3c on 8.7.x 
    Issue #3038062 by Gábor Hojtsy, alexpott: [Symfony 4] Skip...

Status: Fixed » Closed (fixed)

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