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.
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.
> 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.
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.
(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 ?
(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.
(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.
(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.
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.
(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.
Created attachment 226587 [details] Patch
(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.
(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
(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.
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
(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.
(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.
What do we do with this then? Are you all OK with the change?
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.
Created attachment 228036 [details] Patch
(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.
Comment on attachment 228036 [details] Patch Clearing flags on attachment: 228036 Committed r166405: <http://trac.webkit.org/changeset/166405>
All reviewed patches have been landed. Closing bug.