request_uri() creates a request_uri, either directly from apache or by using other values from the super global _SERVER array.

This method is designed to return an abs_path as per http://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html#sec5.1.2 though as noted, proxies must use the absoluteURI form, and are not allowed to rewrite this.

This method blindly assumes that $_SERVER['REQUEST_URI'] returns abs_path and therefore things like form actions are created with urls such as /http://www.example.org/node/86

The enclosed patch checks $_SERVER['REQUEST_URI'] for the absoluteURI form, and then uses parse_url if required to create an abs_path, as expected

CommentFileSizeAuthor
#4 request_uri_5.patch932 bytesAaronBauman
request_uri.patch688 bytesagentblueuk
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

agentblueuk’s picture

agentblueuk CreditAttribution: agentblueuk commented
Version: 6.15 » 7.x-dev

appears to be the same in 7.x-dev too

agentblueuk’s picture

agentblueuk CreditAttribution: agentblueuk commented
Status: Active » Needs review
MichaelCole’s picture

MichaelCole CreditAttribution: MichaelCole commented

request_uri.patch queued for re-testing.

AaronBauman’s picture

AaronBauman
he/him
CreditAttribution: AaronBauman commented
FileSize
request_uri_5.patch932 bytes

ereg() is overkill (and deprecated in PHP 5.3+) -- stripos() is sufficient in this case.

rerolled @ HEAD, no other changes

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
Status: Needs review » Needs work

Ugh. I wonder how that stayed unnoticed for so long. I guess that very few proxies are actually forwarding the request URI as an absolute URL. That said, we are clearly violating HTTP/1.1 here.

if(stripos($_SERVER['REQUEST_URI'], "http") === 0) {

^ This is basically substr($_SERVER['REQUEST_URI'], 0, 4) == 'http' and should be rewritten this way (also, the if and the parenthesis must be separated by a space as per our coding standards). Anyone care to reroll?

pukku’s picture

pukku CreditAttribution: pukku commented

Is there any news on this patch being accepted? Apparently, this needs to get into 7 before it can get into 6?

This bit me today.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Issue summary: View changes

I encountered the leading slash in form action uri with a Druapl 6 and Drupal 7 installation running in Windows 7 recently. My comment #4 in https://drupal.org/node/940470 gives another useable solution to the erroneous leading slash problem caused in the request_uri() function of the boostrap.inc include file.

My understanding is that the use of ltrim to remove the leading slash then prefix the resulting uri with another slash - regardless of whether the uri is an absolute or relative path - is the cause of the problem, and can be easily fixed with the use of preg_replace().