drupal_http_request() does handle (invalid) non-absolute redirects (RFC 7231)

This looks straightforward and logical, BUT:

1. HTTP 1.1 defines Location to be an absolute URI, so it should be that: http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.30
2. Even if it could be a relative URI, if we don't find a protocol, we should only use the path from the response IMHO, as returning a domain and a path without a protocol seems like an extremely broken response.

To clarify (2): Now you use the original protocol if a new one was not provided, use the original domain if a new one was not provided (what about the port??), and you use the path if a new one was not provided. We'd like to support cases where only a path was provided. Is is realistic to support the case when a domain and a path was provided, but not a protocol? That was the concern.

- We'll want to document this better in the code. Please sprinkle with some code comments.

- I wonder if that code is secure. Can we use the raw data like that?

subscribing

Bumping to 7.x now.

Uhhh yeah that's also one of my problems - I expected this issue when I started developing of Linkchecker module with drupal_http_request() function, but haven't yet found the time to test. Thank you very much for saving time.

@AjK: Are you working on the patch? We should also backport this patch to D5 and D6 asap.

I have found another funny server with it's own rules...

--00:00:00--  http://www.oldiemuseum.de/
           => `index.html'
Resolving www.oldiemuseum.de... 212.80.228.99
Connecting to www.oldiemuseum.de|212.80.228.99|:80... connected.
HTTP request sent, awaiting response... 302 Found
Location: ../cms [following]
--00:00:00--  http://www.oldiemuseum.de/../cms
           => `cms'
Connecting to www.oldiemuseum.de|212.80.228.99|:80... connected.
HTTP request sent, awaiting response... 400 Bad Request
00:00:00 ERROR 400: Bad Request.

Drat, forgot to mark as needs review.

If you are adding support for Location: /path you probably need to make a full path handler as if you are allowing values that don't respect the standard you should support other relative paths such as Location: ../path and Location: ./path. This is possible, but as an alternative we can just return an error when the redirect isn't valid.

Marked #1212740: Relative URIs as redirects as duplicate.

Can someone verify if Guzzle has support for this? Than we can move the case back to D7.

https://github.com/guzzle/guzzle/blob/master/src/Guzzle/Http/RedirectPlu...

Cool, how about HTTPRL?

Works in HTTPRL as well :)
http://drupalcode.org/project/httprl.git/blob/d023e1f82c18df277f31f4532b...

Thanks. I will update linkchecker readme to point this out. Have you seen #10? Codewise HTTPRL looks not able to resolve this, but it maybe really an edge case that we don't need to worry about.

Reroll for D7

Fixed tests.

I think we should remove failed tests, because those tests suppose that we can't use relative urls.

Coming here from #2164219: Meta: Feeds "looking" broken due to -1002 missing schema..

Attached a simpler patch.

Oops, fix typo in variable name.

... and correct status.

Adapting tests to the change

This patch should fix the failing tests.

Tests are passing :)

This looks not like a correct patch as it ignores the 3 existing cases.

This get's broken by latest patch (status -1002 is no longer tested):

function system_test_redirect_noscheme() {
  header("Location: localhost/path", TRUE, 301);
  exit;
}

New examples for case #164365:

function system_test_redirect_relative_path() {
  header("Location: ../path", TRUE, 301);
  exit;
}

function system_test_redirect_relative_path_traversal1() {
  header("Location: ../../../../../path", TRUE, 301);
  exit;
}

function system_test_redirect_relative_path_traversal2() {
  header("Location: ../foo/../../../path", TRUE, 301);
  exit;
}

function system_test_redirect_absolute_path() {
  header("Location: /foo/path", TRUE, 301);
  exit;
}

And we may also need to think about protocol relative urls. I believe a fallback to http:// should be useful or we need a new parameter like default_scheme.

Adding the specific RFC keyword since it was missing in my search results

Following patch does not address @hass's remarks. It's a simple reroll of #34 so the patch can be applied on Drupal 7.67.

Same patch, fixed filename...

This bug affects Drupal.org too, for example Planet Drupal, which uses the Aggregator module.