The change introduced by #2555649: drupal_http_request() should allow caller to set the Host header broke drupal_http_request in Drupal 7.61.

The change does not properly handle redirects to different hosts.

The issue is that if a redirect occurs to a different host the 'Host' header field will still contain the original host from the first request ($options is passed back into drupal_http_request when following the redirect). So with this change it sees that the 'Host' header field is populated (with the old host information from the first request), and it fails to set the 'Host' header field to the new host we redirected to. This results in an error. If I revert the change from this patch everything works fine.

I can suggest some solutions but wanted to point out this was a problem and needs to be fixed. I have reverted this patch on my site to restore proper functionality.

CommentFileSizeAuthor
#2 remove_host_on_redirect-3018637-1.patch593 bytesemilymoi
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

emilymoi created an issue. See original summary.

emilymoi’s picture

emilymoi CreditAttribution: emilymoi commented
Status: Active » Needs review
FileSize
remove_host_on_redirect-3018637-1.patch593 bytes

A patch with my suggested fix is attached. I believe unsetting the 'Host' header on redirect is the smallest change possible to fix this issue.

emilymoi’s picture

emilymoi CreditAttribution: emilymoi commented
Version: 7.61 » 7.x-dev
olli’s picture

olli CreditAttribution: olli commented
Status: Needs review » Reviewed & tested by the community
Parent issue: » #2555649: drupal_http_request() should allow caller to set the Host header

With drupal-7.61 the value of drupal_http_request('http://drupal.org/rss.xml')->code is 301, with patch applied it is 200 like it was with drupal-7.60.

tatarbj’s picture

tatarbj
Primary language Hungarian
Location Brussels
CreditAttribution: tatarbj at Petend for European Commission and European Union Institutions, Agencies and Bodies commented
Related issues: +#2986754: drupal_http_request() not properly handling Host header on cross host redirect

I've closed the #2986754: drupal_http_request() not properly handling Host header on cross host redirect in favor of pushing this issue forward as the other one was reporting a bit different bug, but basically, the fix is the same and would be nice to see it in the next D7 minor :)
The fix that is posted here is identical then the one under the other which had 'needs review' status, but now it's closed as duplicated.
Thanks @emilymoi to the cross-checks!
Bests,
Balazs.

tatarbj’s picture

tatarbj
Primary language Hungarian
Location Brussels
CreditAttribution: tatarbj at Petend for European Commission and European Union Institutions, Agencies and Bodies commented

When it will be closed, please credit @das-peter too - https://www.drupal.org/u/das-peter !

ben.kyriakou’s picture

ben.kyriakou CreditAttribution: ben.kyriakou as a volunteer commented

I've also experienced issues because of the previous change, and would appreciate this fix being committed to core.

ben.kyriakou’s picture

ben.kyriakou CreditAttribution: ben.kyriakou as a volunteer commented

I've created a small module that overrides the drupal_http_request function for my own use - may be of interest to other who'd prefer not to patch core. See https://www.drupal.org/project/http_request_redirect_fix

Rhane’s picture

Rhane CreditAttribution: Rhane commented

Any idea of when this will be made a part of core? It does break behaviors for us as well in sites I'm working with. Thank you Emily for noticing this early on! I've only noticed this now due to these updates finally being done to our application (.61 was not a security update I believe)

Fabianx credited das-peter.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented
Assigned: Unassigned » Pol
Issue tags: +Drupal bugfix target, +Pending Drupal 7 commit

Yes, agree that this is a regression.

Assigning to Pol for committing.

Pol’s picture

Pol
the/that
Location Brussels
CreditAttribution: Pol as a volunteer commented

Taking care of this right now.

Pol’s picture

Pol
the/that
Location Brussels
CreditAttribution: Pol as a volunteer commented
Title: The drupal_http_request change that went into Drupal 7.61 is broken » [regression] Unset the 'host' header in drupal_http_request() during redirect.

Updating the title.

  • Pol committed 4d0bb65 on 7.x 
    Issue #3018637 by emilymoi, das-peter: [regression] Unset the 'host'...
Pol’s picture

Pol
the/that
Location Brussels
CreditAttribution: Pol as a volunteer and at Arhs for European Commission and European Union Institutions, Agencies and Bodies commented
Status: Reviewed & tested by the community » Fixed

Fixed!

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented

I think we can make a release for this on the first Wednesday of February if Pol agrees.

tatarbj’s picture

tatarbj
Primary language Hungarian
Location Brussels
CreditAttribution: tatarbj at Petend for European Commission and European Union Institutions, Agencies and Bodies commented

As in the commit it might have been overlooked, I think @olli and me also deserve credits for our testing activities that allowed to resolve this regression :)
Thanks for the commit and your work Pol and Fabian!
Bests,
Balazs.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented

That's fine, next time please include some detailed things you tested per https://www.drupal.org/core/maintainers/issue-credit

Status: Fixed » Closed (fixed)

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

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented
Issue tags: -Drupal bugfix target, -Pending Drupal 7 commit

This is fixed, thanks again everyone!

Removing the pending commit tag (it was already committed).