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.
Comment | File | Size | Author |
---|---|---|---|
#2 | remove_host_on_redirect-3018637-1.patch | 593 bytes | emilymoi |
Comments
Comment #2
emilymoi CreditAttribution: emilymoi commentedA patch with my suggested fix is attached. I believe unsetting the 'Host' header on redirect is the smallest change possible to fix this issue.
Comment #3
emilymoi CreditAttribution: emilymoi commentedComment #4
olli CreditAttribution: olli commentedWith 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.Comment #5
tatarbjI'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.
Comment #6
tatarbjWhen it will be closed, please credit @das-peter too - https://www.drupal.org/u/das-peter !
Comment #7
ben.kyriakou CreditAttribution: ben.kyriakou as a volunteer commentedI've also experienced issues because of the previous change, and would appreciate this fix being committed to core.
Comment #8
ben.kyriakou CreditAttribution: ben.kyriakou as a volunteer commentedI'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_fixComment #9
Rhane CreditAttribution: Rhane commentedAny 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)
Comment #11
Fabianx CreditAttribution: Fabianx as a volunteer commentedYes, agree that this is a regression.
Assigning to Pol for committing.
Comment #12
PolTaking care of this right now.
Comment #13
PolUpdating the title.
Comment #15
PolFixed!
Comment #16
Fabianx CreditAttribution: Fabianx as a volunteer commentedI think we can make a release for this on the first Wednesday of February if Pol agrees.
Comment #17
tatarbjAs 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.
Comment #18
Fabianx CreditAttribution: Fabianx as a volunteer commentedThat's fine, next time please include some detailed things you tested per https://www.drupal.org/core/maintainers/issue-credit
Comment #20
joseph.olstadThis is fixed, thanks again everyone!
Removing the pending commit tag (it was already committed).