Bug 129681

Summary:

[GTK] Too many redirects visiting www.globalforestwatch.org

Product:

WebKit

Reporter:

Sergio Villar Senin <svillar>

Component:

WebKitGTK

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bunhere, commit-queue, dpino, gustavo, gyuyoung.kim, mrobinson, rakuco, sergio, webkit-ews

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

URL:

http://www.globalforestwatch.org

See Also:

https://bugs.webkit.org/show_bug.cgi?id=142074

Attachments:

Description	Flags
http://www.globalforestwatch.org/ loaded in MiniBrowser using Chrome's UA.	none
Loading page in Firefox using MiniBrowser's UA.	none
Patch	none
Patch	none

Sergio Villar Senin

Reported 2014-03-04 04:58:41 PST

It actually redirects first to http://www.globalforestwatch.org/notsupportedbrowser and then the error happens. Maybe it's just an UA issue + some problem in the server, but we need to double check it.

Attachments
http://www.globalforestwatch.org/ loaded in MiniBrowser using Chrome's UA. (592.06 KB, image/png) 2014-03-12 09:04 PDT, Diego Pino	no flags	Details
Loading page in Firefox using MiniBrowser's UA. (29.13 KB, image/png) 2014-03-13 01:44 PDT, Diego Pino	no flags	Details
Patch (3.65 KB, patch) 2014-03-13 03:14 PDT, Diego Pino	no flags	Details Formatted Diff Diff
Patch (3.19 KB, patch) 2014-03-28 04:02 PDT, Diego Pino	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Diego Pino

Comment 1 2014-03-12 07:49:10 PDT

I monitored the request with Wireshark and this is the output: GET / HTTP/1.1 Host: www.globalforestwatch.org Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X) AppleWebKit/537.30 (KHTML, like Gecko) Safari/537.30 Accept-Encoding: gzip, deflate Accept-Language: en-us Connection: Keep-Alive HTTP/1.1 302 Found Cache-Control: no-cache Content-Encoding: gzip Content-Type: text/html; charset=utf-8 Date: Wed, 12 Mar 2014 14:32:27 GMT Location: http://www.globalforestwatch.org/notsupportedbrowser Server: WEBrick/1.3.1 (Ruby/2.1.0/2013-12-25) Vary: Accept-Encoding X-Frame-Options: ALLOWALL X-Request-Id: e8a845bb-7bad-4e82-b35f-41132603e6a9 X-Runtime: 0.004316 Content-Length: 123 Connection: keep-alive .....o S....A.. ......@.....X...kI.i....s.l.....;.r.R................c.....<....E+.b..C..d...KR....'.J..-........... .3v...GET /notsupportedbrowser HTTP/1.1 Host: www.globalforestwatch.org Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X) AppleWebKit/537.30 (KHTML, like Gecko) Safari/537.30 Accept-Encoding: gzip, deflate Accept-Language: en-us Connection: Keep-Alive HTTP/1.1 302 Found Cache-Control: no-cache Content-Encoding: gzip Content-Type: text/html; charset=utf-8 Date: Wed, 12 Mar 2014 14:32:27 GMT Location: http://www.globalforestwatch.org/notsupportedbrowser Server: WEBrick/1.3.1 (Ruby/2.1.0/2013-12-25) Vary: Accept-Encoding X-Frame-Options: ALLOWALL X-Request-Id: 24c51c74-bc63-4258-9639-5c991298c905 X-Runtime: 0.004940 Content-Length: 123 Connection: keep-alive .....o S....A.. ......@.....X...kI.i....s.l.....;.r.R................c.....<....E+.b..C..d...KR....'.J..-........... .3v...GET /notsupportedbrowser HTTP/1.1 Host: www.globalforestwatch.org Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X) AppleWebKit/537.30 (KHTML, like Gecko) Safari/537.30 Accept-Encoding: gzip, deflate Accept-Language: en-us Connection: Keep-Alive And this keeps going on and on until the "Too many redirects" error happen. I think this a problem related to the site. If I request the root page using curl: $ curl http://www.globalforestwatch.org <html><body>You are being <a href="http://www.globalforestwatch.org/notsupportedbrowser">redirected</a>.</body></html> If now I follow the redirection and request "http://www.globalforestwatch.org/notsupportedbrowser": $ curl http://www.globalforestwatch.org/notsupportedbrowser <html><body>You are being <a href="http://www.globalforestwatch.org/notsupportedbrowser">redirected</a>.</body></html> It seems clear that "http://www.globalforestwatch.org/notsupportedbrowser" should not check whether the browser is supported or not, because otherwise it creates a redirection infinite loop.

Sergio Villar Senin

Comment 2 2014-03-12 08:46:47 PDT

> It seems clear that "http://www.globalforestwatch.org/notsupportedbrowser" should not check whether the browser is supported or not, because otherwise it creates a redirection infinite loop. I mean, the site clearly has some kind of server side issue. What I'd like to know is if there is some mistake by our side. One interesting test would be to change the UA so that we claim to be one of the supported browsers and check what happens.

Diego Pino

Comment 3 2014-03-12 09:04:33 PDT

Created attachment 226512 [details] http://www.globalforestwatch.org/ loaded in MiniBrowser using Chrome's UA. I changed the UA of MiniBrowser to the UA of Chrome (which works fine). "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/32.0.1700.102 Safari/537.36" The page was successfully loaded. Snapshot attached.

Sergio Villar Senin

Comment 4 2014-03-12 09:24:44 PDT

(In reply to comment #3) > Created an attachment (id=226512) [details] > http://www.globalforestwatch.org/ loaded in MiniBrowser using Chrome's UA. > > I changed the UA of MiniBrowser to the UA of Chrome (which works fine). > > "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/32.0.1700.102 Safari/537.36" > > The page was successfully loaded. Snapshot attached. Great, what if you directly visit http://www.globalforestwatch.org/notsupportedbrowser ?

Diego Pino

Comment 5 2014-03-12 09:34:29 PDT

(In reply to comment #4) > Great, what if you directly visit http://www.globalforestwatch.org/notsupportedbrowser ? If I load "http://www.globalforestwatch.org/notsupportedbrowser" in MiniBrowser using Chrome's UA, the page is loaded successfully. I also tested it in Safari as I thought that as MiniBrowser uses the same UA as Safari it should fail on Safari too. However in Safari it worked. Why is that? The difference between MiniBrowser UA and Safari UA is that Safari UA has a "Version" string. For instance: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_8) AppleWebKit/537.13+ (KHTML, like Gecko) Version/5.1.7 Safari/534.57.2" I pressume that in the case of detecting the browser is a Safari, the code that checks the browser support may try to fetch the version string too, as not all versions of Safari might be supported by this site. In fact, just by editing MiniBrowser UA and add "Version/6.0", the page gets loaded. If I try doing the same using "Version/3.0" the "too many redirects" error happens.

Sergio Villar Senin

Comment 6 2014-03-12 10:19:48 PDT

(In reply to comment #5) > (In reply to comment #4) > > > Great, what if you directly visit http://www.globalforestwatch.org/notsupportedbrowser ? > > If I load "http://www.globalforestwatch.org/notsupportedbrowser" in MiniBrowser using Chrome's UA, the page is loaded successfully. > > I also tested it in Safari as I thought that as MiniBrowser uses the same UA as Safari it should fail on Safari too. However in Safari it worked. Why is that? The difference between MiniBrowser UA and Safari UA is that Safari UA has a "Version" string. For instance: > > "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_8) AppleWebKit/537.13+ (KHTML, like Gecko) Version/5.1.7 Safari/534.57.2" > > I pressume that in the case of detecting the browser is a Safari, the code that checks the browser support may try to fetch the version string too, as not all versions of Safari might be supported by this site. > > In fact, just by editing MiniBrowser UA and add "Version/6.0", the page gets loaded. If I try doing the same using "Version/3.0" the "too many redirects" error happens. Awesome report. Martin, Gustavo what do you think about tunning a bit our UA? This time isn't about replacing Chrome by Safari or something but just adding the "Version" thing Diego mentioned. It looks like it isn't only about this site, but I get many "unsupported browser" messages on sites that work fine with Safari versions older than my builds.

Martin Robinson

Comment 7 2014-03-12 10:42:35 PDT

(In reply to comment #6) > Awesome report. Martin, Gustavo what do you think about tunning a bit our UA? This time isn't about replacing Chrome by Safari or something but just adding the "Version" thing Diego mentioned. Sounds like we need to add the version string. I think another interesting test is what happens to other browsers when you give them the bad UA. If they don't go into the redirect loop, this is actually two bugs.

Diego Pino

Comment 8 2014-03-13 01:44:13 PDT

Created attachment 226584 [details] Loading page in Firefox using MiniBrowser's UA. As Martin suggested, I tried to load the page in Firefox but using MiniBrowser's UA (I used the User Agent Switcher extension to do that). The result is an error stating that the page isn't redirecting properly. Snapshot attached.

Sergio Villar Senin

Comment 9 2014-03-13 02:05:46 PDT

(In reply to comment #8) > Created an attachment (id=226584) [details] > Loading page in Firefox using MiniBrowser's UA. > > As Martin suggested, I tried to load the page in Firefox but using MiniBrowser's UA (I used the User Agent Switcher extension to do that). The result is an error stating that the page isn't redirecting properly. Snapshot attached. Yeah then pretty sure they have some server side issue. Let's add the version string then. Perhaps we should use the one from the latest release of Safari.

Diego Pino

Comment 10 2014-03-13 03:14:23 PDT

Created attachment 226587 [details] Patch

Martin Robinson

Comment 11 2014-03-13 07:34:43 PDT

(In reply to comment #9) > (In reply to comment #8) > > Created an attachment (id=226584) [details] [details] > > Loading page in Firefox using MiniBrowser's UA. > > > > As Martin suggested, I tried to load the page in Firefox but using MiniBrowser's UA (I used the User Agent Switcher extension to do that). The result is an error stating that the page isn't redirecting properly. Snapshot attached. > > Yeah then pretty sure they have some server side issue. Let's add the version string then. Perhaps we should use the one from the latest release of Safari. Sounds like we also need some infinite redirect protection.

Sergio Villar Senin

Comment 12 2014-03-13 08:08:16 PDT

(In reply to comment #11) > (In reply to comment #9) > > (In reply to comment #8) > > > Created an attachment (id=226584) [details] [details] [details] > > > Loading page in Firefox using MiniBrowser's UA. > > > > > > As Martin suggested, I tried to load the page in Firefox but using MiniBrowser's UA (I used the User Agent Switcher extension to do that). The result is an error stating that the page isn't redirecting properly. Snapshot attached. > > > > Yeah then pretty sure they have some server side issue. Let's add the version string then. Perhaps we should use the one from the latest release of Safari. > > Sounds like we also need some infinite redirect protection. What do you mean? We already show a "Too many redirects" error page

Martin Robinson

Comment 13 2014-03-13 08:16:36 PDT

(In reply to comment #12) > What do you mean? We already show a "Too many redirects" error page Oh, right! Sorry. I wasn't thinking properly.

Gustavo Noronha (kov)

Comment 14 2014-03-14 11:47:45 PDT

I agree. Another thing that we should look into is convincing google to give us some features it's not delivering like image search where you upload an image to search similars. It does if we say we're Chromium. boo

Diego Pino

Comment 15 2014-03-14 16:29:25 PDT

(In reply to comment #11) > Sounds like we also need some infinite redirect protection. What it might be interesting is to implement a mechanism to detect infinite redirection loops (at least the simplest case, which is a page redirecting to itself). This way we can avoid the all the unnecessary redirections until the maximum limit is reached.

Martin Robinson

Comment 16 2014-03-14 16:30:41 PDT

(In reply to comment #15) > (In reply to comment #11) > > > Sounds like we also need some infinite redirect protection. > > What it might be interesting is to implement a mechanism to detect infinite redirection loops (at least the simplest case, which is a page redirecting to itself). This way we can avoid the all the unnecessary redirections until the maximum limit is reached. I think that would be very difficult. A page might serve different content from the same URL.

Sergio Villar Senin

Comment 17 2014-03-25 06:55:32 PDT

What do we do with this then? Are you all OK with the change?

Martin Robinson

Comment 18 2014-03-25 09:24:36 PDT

Comment on attachment 226587 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226587&action=review Did you run unit tests. I think some of them check the expected user agent. > Source/cmake/OptionsGTK.cmake:149 > +add_definitions(-DUSER_AGENT_SAFARI_MAJOR_VERSION=7) > +add_definitions(-DUSER_AGENT_SAFARI_MINOR_VERSION=0) Not sure how I feel about adding more command-line arguments to the compiler.

Diego Pino

Comment 19 2014-03-28 04:02:39 PDT

Created attachment 228036 [details] Patch

Diego Pino

Comment 20 2014-03-28 04:07:08 PDT

(In reply to comment #18) > (From update of attachment 226587 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226587&action=review > > Did you run unit tests. I think some of them check the expected user agent. > Run master, Version/6.0 and Version/7.0. Here are the results: Master: * Tests that timed out (15): /WebKit2Gtk/TestWebKitWebView, /WebKit2Gtk/TestCookieManager, /WebKit2Gtk/TestWebExtensions, /WebKit2Gtk/TestWebKitWebViewGroup, /WebKit2Gtk/TestWebKitPolicyClient, /WebKit2Gtk/TestMultiprocess, /WebKit2Gtk/TestWebKitSettings, /WebKit2Gtk/TestWebKitAccessibility, /WebKit2Gtk/TestLoaderClient, /WebKit2Gtk/TestPrinting, /WebKit2Gtk/TestDOMNode, /WebKit2Gtk/TestFrame, /WebKit2Gtk/TestBackForwardList, /WebKit2Gtk/TestWebKitWebContext, /WebKitGtk/testwebinspector * Tests skipped (23). Version/6.0: * Tests that timed out (13): /WebKit2Gtk/TestWebKitWebView, /WebKit2Gtk/TestCookieManager, /WebKit2Gtk/TestSSL, /WebKit2Gtk/TestWebExtensions, /WebKit2Gtk/TestWebKitWebViewGroup, /WebKit2Gtk/TestWebKitPolicyClient, /WebKit2Gtk/TestMultiprocess, /WebKit2Gtk/TestPrinting, /WebKit2Gtk/TestDOMNode, /WebKit2Gtk/TestFrame, /WebKit2Gtk/TestBackForwardList, /WebKit2Gtk/TestWebKitWebContext, /WebKitGtk/testwebinspector * Tests skipped (23). Version/7.0: * Tests that timed out (16): /WebKit2Gtk/TestWebKitWebView, /WebKit2Gtk/TestCookieManager, /WebKit2Gtk/TestSSL, /WebKit2Gtk/TestWebExtensions, /WebKit2Gtk/TestWebKitWebViewGroup, /WebKit2Gtk/TestWebKitPolicyClient, /WebKit2Gtk/TestMultiprocess, /WebKit2Gtk/TestWebKitSettings, /WebKit2Gtk/TestWebKitAccessibility, /WebKit2Gtk/TestLoaderClient, /WebKit2Gtk/TestPrinting, /WebKit2Gtk/TestDOMNode, /WebKit2Gtk/TestFrame, /WebKit2Gtk/TestBackForwardList, /WebKit2Gtk/TestWebKitWebContext, /WebKitGtk/testwebinspector * Tests skipped (23). So, in the end I decided to set the version string to Version/6.0. > > Source/cmake/OptionsGTK.cmake:149 > > +add_definitions(-DUSER_AGENT_SAFARI_MAJOR_VERSION=7) > > +add_definitions(-DUSER_AGENT_SAFARI_MINOR_VERSION=0) > > Not sure how I feel about adding more command-line arguments to the compiler. OK, I appended Version/6.0 directly to the UserAgent string.

WebKit Commit Bot

Comment 21 2014-03-28 07:19:12 PDT

Comment on attachment 228036 [details] Patch Clearing flags on attachment: 228036 Committed r166405: <http://trac.webkit.org/changeset/166405>

WebKit Commit Bot

Comment 22 2014-03-28 07:19:17 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.