1387938 - empty Location: http header creates 21 requests to the origin URI (302 redirect)

Status: NEW

(Core :: Networking: HTTP, enhancement, P3)

Description

[Karl Dubost💡 :karlcow]

This is a spin-off of https://webcompat.com/issues/8678

1. With Firefox Nightly, open a new window
2. Open developer tools and the network panel or the console with "Requests" on
3. Go to http://www.1clickurl.com/

What's happening?

http://www.1clickurl.com/css/bootstrap.min.css.map
is accessed multiple times with 302 HTTP response. 


The response headers are:

```http
HTTP/1.1 302 Found
Server: nginx
Date: Mon, 07 Aug 2017 04:26:37 GMT
Content-Type: text/html; charset=UTF-8
Content-Length: 0
Connection: keep-alive
Keep-Alive: timeout=60
X-Powered-By: PHP/5.4.45
Location:
```

Notice the empty location. After 21 times, the browser seems to stop trying.

It might not be necessary to try 21 times, before giving up. :)

Comment 1

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

The limit of redirection cycle is controlled by pref "network.http.redirection-limit", which is set as 20 now.
https://searchfox.org/mozilla-central/rev/b52285fffc13f36eca6b47de735d4e4403b3859e/modules/libpref/init/all.js#1662

Bug 173147 is the history of why we chose 20.

Comment 2

[Karl Dubost💡 :karlcow]

Ah interesting. Thanks Shih-Chiang Chien

The rationale for the 20 limit is sound.

> It seems that nytimes.com has been known to string together more than 10 redirects…

But I suspect it is for really chained redirects, aka things like

example.com -> 302 a.example.com -> 302 b.example.org -> etc.


Here we are in a case of getting the same URL over and over again. 
Should there be a check on the location?

aka 

if http_location_header is None or http_location_header == origin_url:
     Give up.

Comment 3

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Quickly give up when a server tries to redirect to the same URL sounds like a good error handling to me. However I'm not sure if there is any real website doing it on purpose without fixing it. Put this bug in our backlog first.
@jduell do you think this is something we want to fix in release 57 time frame?

Comment 4

[Daniel Stenberg [:bagder]]

Redirecting back to the same URL is not an uncommon practice on the web so preventing that would break a non-insignificant number of sites. Some sites set a cookie (for example) and redirects back to itself to trigger a different code path.

Comment 5

[Karl Dubost💡 :karlcow]

> Redirecting back to the same URL is not an uncommon practice on the web so preventing that would break a non-insignificant number of sites. Some sites set a cookie (for example) and redirects back to itself to trigger a different code path.

Yup. That would be interesting to measure.

Comment 6

[Honza Bambas (:mayhemer)]

(In reply to Daniel Stenberg [:bagder] from comment #4)
> Redirecting back to the same URL is not an uncommon practice on the web so
> preventing that would break a non-insignificant number of sites. Some sites
> set a cookie (for example) and redirects back to itself to trigger a
> different code path.

Confirming.  Don't disallow redir to itself.  

Although, an empty Location is something to investigate how to treat.  We fail when there are two Location headers (sec reasons), we don't redir (actually can't) when it's missing.  My first though is to treat it as missing (i.e. don't redirect) and behave as the redir has been vetoed.  But I have nothing to support it, simply seems logical.

Comment 7

[Karl Dubost💡 :karlcow]

ok let's test the Web Compatibility side of things.


I created locally a server


http GET http://127.0.0.1:8080

HTTP/1.0 302 Found
Content-type: text/plain
Date: Mon, 07 Aug 2017 23:43:29 GMT
Location:
Server: BaseHTTP/0.3 Python/2.7.10



* Firefox Nightly 57: 
  21 requests then giving up

* Safari Release 36 (Safari 11.0, WebKit 12604.1.32.0.1) Tech Preview:
  Blank page
  1 request. Not visible in the network panel. 
  The server received in fact two requests one for /, one for /favicon.ico

* Opera beta 47.0.2631.34 (aka Chrome/60.0.3112.78):
  Blank page
  1 request. visible in the network panel.
  server received two requests to / and /favicon.ico

* httpie 0.9.9: 
  30 requests then giving up (option -F)

* curl 7.54.0: 
  1 request (option -L). Not sure about this one.

Comment 8

[Jason Duell]

This is fairly low priority IMO.  The existing behavior doesn't actually break anything, it only happens when websites are acting stupidly, and potential fixes are going to have to tested live on the web for bustage.

That said, it's possible that a simple patch to fail redirects (per comment 6) when we see an Empty Location: might work here.  I'd feel a lot happier if someone looked at what Chrome's algorithm here is and then we can presumably copy it safely.  Could be worth a little of someone's time when you've got free cycles.
 
Karl, can you test Chrome's behavior while you've got your server set up?

Comment 9

[Karl Dubost💡 :karlcow]

Chrome should give more or less the same results than Opera but for the sake of it.

* Chrome Version 60.0.3112.90 (Build officiel) (64 bits)
  Blank page
  1 request. visible in the network panel.
  server received two requests to / and /favicon.ico


Also to be sure I checked for 
Location: http://127.0.0.1:8080

* Firefox: 21 requests
* Safari:  17 requests
* Opera:   63 requests
* Chrome:  63 requests
* httpie:  30 requests
* curl:    47 requests

Apart of Firefox and httpie different results for
a request on http://127.0.0.1:8080 with a 302.

* Location: 
* Location: http://127.0.0.1:8080

Comment 10

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 11

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258