Summary: | Proper handling of HTTP 204 (No Content) status code | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Luiz Agostini <luiz> | ||||
Component: | WebKit Misc. | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | REOPENED --- | ||||||
Severity: | Normal | CC: | ahmad.saleem792, ap, bburg, beidson, croelli, danyao, darin, goran.karlic, gordon.murray, gyuyoung.kim, japhet, kidniki100, koivisto, leandro, mcatanzaro, mike, mrobinson, psolanki, rakuco, sullivan, webkit-bug-importer, webkit.review.bot | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Luiz Agostini
2011-05-04 13:23:28 PDT
Could you please clarify what exactly is the bug to fix here? This report doesn't seem actionable in its current state. (In reply to comment #1) > Could you please clarify what exactly is the bug to fix here? This report doesn't seem actionable in its current state. Responses containing HTTP code 204 should be ignored by the browser and the view should not be updated. I made it for Qt and added a test for it. The new test is http/tests/navigation/response204.html. Some platforms (Mac, Win and GTK) fail on this new test. But it seems that if this test fail in Mac or Win then hundreds of other tests fail as well. See https://bugs.webkit.org/show_bug.cgi?id=42529#c21 The test was skipped in those platforms by r85784. Here is how I tested: 1. Opened http://www.apple.com in Safari 5.0.5 (also tested with ToT). 2. Once that finished loading, changed address in address bar to <http://127.0.0.1:8000/navigation/resources/response204.pl>, and hit Enter. Results: The address reverted to http://www.apple.com, page content was not affected. This looks like expected behavior. I haven't investigated what's going on in the test, but there are no known issues with 204 responses (we track bug 4858 here, but it's not really about 204, it's about stopping timers too early on any load). (In reply to comment #3) > Here is how I tested: > 1. Opened http://www.apple.com in Safari 5.0.5 (also tested with ToT). > 2. Once that finished loading, changed address in address bar to <http://127.0.0.1:8000/navigation/resources/response204.pl>, and hit Enter. > > Results: The address reverted to http://www.apple.com, page content was not affected. Good. I was expecting this test to fail in some platforms but it seems that Mac is ok with this 204 handling. > > This looks like expected behavior. I haven't investigated what's going on in the test, but there are no known issues with 204 responses (we track bug 4858 here, but it's not really about 204, it's about stopping timers too early on any load). Yeah, it may be then that I did not provide the proper test. The response204.is very simple, I will copy it here: <script> if (window.layoutTestController) { layoutTestController.dumpAsText(); layoutTestController.dumpBackForwardList(); layoutTestController.queueLoad("resources/response204.pl"); } </script> And this is the result that I was expecting: ============== Back Forward List ============== curr-> http://127.0.0.1:8000/navigation/response204.html **nav target** =============================================== Meaning that the response from resources/response204.pl was ignored. But it was not the result for SnowLeopard as you can see here http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20%28Tests%29/r85783%20%2828972%29/http/tests/navigation/response204-diffs.txt : --- /Volumes/Data/WebKit-BuildSlave/snowleopard-intel-release-tests/build/layout-test-results/http/tests/navigation/response204-expected.txt 2011-05-04 13:57:57.000000000 -0700 +++ /Volumes/Data/WebKit-BuildSlave/snowleopard-intel-release-tests/build/layout-test-results/http/tests/navigation/response204-actual.txt 2011-05-04 13:57:57.000000000 -0700 @@ -1,5 +1,6 @@ ============== Back Forward List ============== -curr-> http://127.0.0.1:8000/navigation/response204.html **nav target** + http://127.0.0.1:8000/navigation/response204.html **nav target** +curr-> http://127.0.0.1:8000/navigation/resources/response204.pl **nav target** =============================================== Nor for Leopard as you can see here http://build.webkit.org/results/Leopard%20Intel%20Release%20%28Tests%29/r85783%20%2831877%29/http/tests/navigation/response204-diffs.txt : --- /Volumes/Big/WebKit-BuildSlave/leopard-intel-release-tests/build/layout-test-results/http/tests/navigation/response204-expected.txt 2011-05-04 13:47:14.000000000 -0700 +++ /Volumes/Big/WebKit-BuildSlave/leopard-intel-release-tests/build/layout-test-results/http/tests/navigation/response204-actual.txt 2011-05-04 13:47:14.000000000 -0700 @@ -1,5 +1,6 @@ ============== Back Forward List ============== -curr-> http://127.0.0.1:8000/navigation/response204.html **nav target** + http://127.0.0.1:8000/navigation/response204.html **nav target** +curr-> http://127.0.0.1:8000/navigation/resources/response204.pl **nav target** =============================================== Is that test wrong? BTW, I have tried the test in bug 4858 and did not reproduce the problem in safari 5.0.5 or in chromium 11.0.696.57 or QtTestBrowser r85857. :) I see. Turns out that Safari handles the policy for 204, not WebKit. I think that it should be implemented in WebCore. Seems strange that the test doesn't just use waitUntilDone/window.location="...", but there probably was a reason to use queued load machinery. Thanks for testing bug 4858! I'm surprised that it no longer fails, but it's great news. Safari does currently handle that code directly, but it seems fine to me to handle it in WebKit instead (Darin said the same thing once too). The other question is why so many tests failed after running this test. It seems that if http/tests/navigation/response204.html fails then hundreds of other tests fail as well. https://bugs.webkit.org/show_bug.cgi?id=42529#c21 http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20%28Tests%29/r85783%20%2828972%29/results.html http://build.webkit.org/results/Leopard%20Intel%20Release%20%28Tests%29/r85783%20%2831877%29/results.html *** This bug has been marked as a duplicate of bug 104865 *** Going to reverse the duplication graph here. *** Bug 104865 has been marked as a duplicate of this bug. *** Created attachment 180810 [details]
Patch
As mentioned in duplicate, this change looks good to me, but someone who worked on loader code more recently should review. Comment on attachment 180810 [details]
Patch
Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.
This issue is still reproducable on iOS 10. The easiest way to reproduce is via the site: http://httpstat.us/ When you click the link "204 No Content" on a desktop browser it is handled correctly the browser does nothing to view as per HTTP spec. On iOS the view is changed. Does this patch work? In iOS 10, this is a huge issue and can easily be reproduced while using an Amazon s3 Form post. In every other browser, Chrome on android and all Desktop browsers, the current page view is kept when a 204 response is recieved. In both Chrome for iOS and Safari iOS, it redirects to a blank page, which it should not. Do we have to wait for Apple to update for this to work? Have raised separate Bug 166633 - iOS browsers not correctly handling HTTP 204 response to flag the iOS handling issue. *** Bug 166633 has been marked as a duplicate of this bug. *** I don't think this is a Webkit bug. Firefox on iOS has the exact same issue and it is Gecko based. This is an Apple bug it seems. I don't know what the browsers get from the OS but it seems to be telling them to actually render a blank page. I've tested this in every iOS browser and the same thing occurs, a blank page gets loaded. Comment on attachment 180810 [details]
Patch
Patch looks good to me too. Someone should apply this and test to see if this change fixes things in Safari on iOS (could also try other WebKit-based iOS browsers such as Chrome and Firefox).
(In reply to comment #20) > I don't think this is a Webkit bug. Firefox on iOS has the exact same issue > and it is Gecko based. This is an Apple bug it seems. I don't know what > the browsers get from the OS but it seems to be telling them to actually > render a blank page. I've tested this in every iOS browser and the same > thing occurs, a blank page gets loaded. All web browsers on iOS are WebKit based including Firefox which would normally be based on Gecko. Is this issue going to be resolved? It is an obvious violation of the HTTP standard and inconsistent with all on other platforms, so it should be resolved. |