Show internal error page for HTTP responses with error codes (4xx, 5xx) and "Content-Length: 0" instead of blank page
Categories
(Core :: Networking, defect, P2)
Tracking
()
People
(Reporter: blassey, Assigned: sekim)
References
(Blocks 2 open bugs)
Details
(Keywords: parity-chrome, perf-alert, Whiteboard: [necko-triaged][necko-priority-queue])
Attachments
(2 files, 4 obsolete files)
Reporter | ||
Updated•9 years ago
|
Comment 1•9 years ago
|
||
Comment 2•9 years ago
|
||
Comment 3•9 years ago
|
||
Comment 4•9 years ago
|
||
Reporter | ||
Updated•9 years ago
|
Comment 6•9 years ago
|
||
Comment 7•9 years ago
|
||
Comment 8•9 years ago
|
||
Comment 9•8 years ago
|
||
Updated•3 years ago
|
Using this bug to track showing error pages on http error responses with "Content-Length: 0", because it is the oldest I could find. Is there any specification which says that the browser should or may show an internal error page for http errors? In any case I think it would be less confusing for users instead of seeing an empty page.
Updated•2 years ago
|
Assignee | ||
Comment 15•1 year ago
•
|
||
Was able to reproduce this issue with:
http.server import HTTPServer, BaseHTTPRequestHandler
class TestHandler(BaseHTTPRequestHandler):
def do_GET(self):
self.send_response(500)
self.send_header("Content-Length", 0)
self.end_headers()
if __name__ == "__main__":
server_address = ('', 8000)
httpd = HTTPServer(server_address, TestHandler)
httpd.serve_forever()
Marking as P2 since it is relatively easy to fix this problem.
Assignee | ||
Updated•1 year ago
|
Assignee | ||
Comment 16•1 year ago
|
||
Comment 17•11 months ago
•
|
||
I found this change breaks session history when navigating to a page with HTTP status 500 and Content-Length: 0
.
STR:
- Open https://emk.name/test/500download.cgi in a new tab with the patched build.
- Click the "500 zero byte response" link.
AR:
Back button will be disabled.
ER:
Back button should be enabled.
This bug already happens with downloads with HTTP status 500. (Try "500 download" link above).
Assignee | ||
Updated•11 months ago
|
Assignee | ||
Comment 18•11 months ago
|
||
Assignee | ||
Comment 19•11 months ago
|
||
The updated patch now should have back button enabled unless we do something like ./mach run https://emk.name/test/500download.cgi
Comment 20•11 months ago
|
||
The latest patch didn't fix the problem at all.
unless we do something like
./mach run https://emk.name/test/500download.cgi
Why did you exclude it arbitrarily? It is the exact case I want to fix. Did you notice a phrase "in a new tab" in STR?
Moreover, even if we exclude that case, the patch still breaks the session history.
STR:
- Open https://bugzilla.mozilla.org/show_bug.cgi?id=1325876 (this bug).
- Click a link in this bug to navaigate to https://emk.name/test/500download.cgi.
- Click the "500 zero byte response" link.
- Click the back button.
AR:
Back to https://bugzilla.mozilla.org/show_bug.cgi?id=1325876.
ER:
Back to https://emk.name/test/500download.cgi.
I showed a new tab case just because the bug was visually obvious. The underlying problem is present regardless of a new tab is opened or not.
Assignee | ||
Comment 21•11 months ago
•
|
||
Currently investigating the second STR.
I did further investigations and the first STR is an issue that persists on other errors (e.g. NS_ERROR_NET_ERROR_RESPONSE
)
This was reproducible on other error pages with:
- Run this Python script.
from http.server import HTTPServer, BaseHTTPRequestHandler
class TestHandler(BaseHTTPRequestHandler):
def do_GET(self):
if self.path == '/test':
self.send_response(500)
self.send_header("Content-Length", "0")
self.end_headers()
else:
self.send_response(200)
self.send_header("Content-type", "text/html")
self.end_headers()
html_content = """
<html>
<head>
<title>Test Page</title>
</head>
<body>
<a href="/test">HTTP 500 and unusual content-type</a>
</body>
</html>
"""
self.wfile.write(html_content.encode('utf-8'))
if __name__ == "__main__":
server_address = ('', 8000)
httpd = HTTPServer(server_address, TestHandler)
httpd.serve_forever()
-
Build this patch and run
./mach run localhost:8000
-
Navigate to "HTTP 500 and unusual content-type"
Expected: Back button is enabled.
Actual: Back button is not enabled.
I think we should create a separate bug to handle session history.
Assignee | ||
Updated•11 months ago
|
Assignee | ||
Comment 22•11 months ago
|
||
Assignee | ||
Comment 23•11 months ago
|
||
Assignee | ||
Updated•11 months ago
|
Assignee | ||
Updated•11 months ago
|
Assignee | ||
Comment 24•11 months ago
|
||
Updated•10 months ago
|
Assignee | ||
Updated•8 months ago
|
Assignee | ||
Updated•8 months ago
|
Comment 26•8 months ago
|
||
Comment 27•8 months ago
|
||
Comment 28•8 months ago
|
||
Backed out for causing multiple failures.
- Backout link
- Push with failures all failures
————————————————————————————————————
- Push with failures wpt failures
- Failure Log
————————————————————————————————————
- Push with failures mochitest failures @test_http_background_auth_request.html
- Failure Log
————————————————————————————————————
- Push with failures bc failures
- Failure Log
Comment 29•8 months ago
|
||
Assignee | ||
Updated•8 months ago
|
Comment 31•8 months ago
•
|
||
This also breaks ui-test-arm
jobs on CI in [1] verifyFirefoxSuggestHeaderForBrowsingDataSuggestionsTest
and [2] verifyTabsSearchWithOpenTabsTest
which load generic assets through a local web server (OK HTTP MockWebServer) in Fenix tests. The tabs are showing a 'Cannot Complete Request' error. Fenix is expected to show the name of the page (e.g, generic1.html) (loaded via local web server) instead of this error as a search suggestion.
[1] https://searchfox.org/mozilla-central/source/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/ui/SearchTest.kt#672
[2] https://searchfox.org/mozilla-central/source/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/ui/SearchTest.kt#758
- CI logs: https://treeherder.mozilla.org/logviewer?job_id=487147286&repo=autoland&lineNumber=3385
- Firebase Test Lab matrix link: https://console.firebase.google.com/project/moz-fenix/testlab/histories/bh.66b7091e15d53d45/matrices/6498108635348826662/details?stepId=bs.c6325c2449c71a89&testCaseId=4
Android APK: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/Wv5filItRUOz6kg9pVZ2dg/artifacts/public/build/target.arm64-v8a.apk
Android test APK: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/ODTdg2rpTpOky1hrLjGUGA/artifacts/public/build/target.noarch.apk
Reproducible locally via (after installing both APKs on device):
adb shell am instrument -w -e class org.mozilla.fenix.ui.SearchTest#verifyTabsSearchWithOpenTabsTest org.mozilla.fenix.debug.test/androidx.test.runner.AndroidJUnitRunner
Matrix has videos and screenshots
Assignee | ||
Comment 32•8 months ago
•
|
||
Disabling below chunk of code strangely fixed the problem on Android (using #ifndef ANDROID
).. Seems like generic1.html has a contentLength
of 0 then returns responseCode
above 400 (confirmed that rv
did not fail). Also confirmed that the error passed here is ERROR_UNKNOWN
on Fenix.
if (NS_FAILED(rv) || contentLength <= 0) {
if (responseCode >= 500) {
return NS_ERROR_NET_ERROR_RESPONSE;
}
if (responseCode >= 400) {
return NS_ERROR_NET_EMPTY_RESPONSE;
}
}
Responsible patch: https://phabricator.services.mozilla.com/D220193
Comment 33•8 months ago
|
||
Comment 34•8 months ago
•
|
||
Backed out for causing bc failures on browser_subdocument_downgrade.js
This failure is also caused by this bug.
Assignee | ||
Comment 35•8 months ago
|
||
Updated•8 months ago
|
Comment 36•8 months ago
|
||
Comment 37•8 months ago
•
|
||
Backed out for causing multiple failures.
- Push with failures - wpt failures
- Failure Log
- Failure line: TEST-UNEXPECTED-ERROR | /loading/early-hints/404-with-early-hints.h2.window.html | Reached error page: about:neterror?e=httpErrorPage&u=https%3A//web-platform.test%3A9000/loading/early-hints/resources/404-with-early-hints.h2.py%3Fresource-url%3Dhttps%253A%252F%252Fweb-platform.test%253A9000%252Floading%252Fearly-hints%252Fresources%252Fsquare.png%253F7d1545a6-6f8c-4cf4-a293-a78777afebd8&c=UTF-8&d=https%3A//web-platform.test%3A9000/loading/early
- Push with failures - mochitests failures
- Failure Log
- Failure line: TEST-UNEXPECTED-FAIL | browser/components/tabbrowser/test/browser/tabs/browser_link_in_tab_title_and_url_prefilled_normal_page_blank_target.js | Test timed out -
- Push with failures - another wpt failures
- Failure Log
- Failure line: TEST-UNEXPECTED-TIMEOUT | /resource-timing/response-status-code.html | This test validates the response status of resources. 62 - Test timed out
Comment 38•8 months ago
|
||
Comment 39•8 months ago
•
|
||
Backed out for causing wd fails @ navigation.spec.js
- Backout link
- Push with failures
- Failure Log
- Failure line:
TEST-UNEXPECTED-FAIL | navigation Page.goto should not throw an error for a 404 response with an empty body (navigation.spec.js) | expected PASS
TEST-UNEXPECTED-FAIL | navigation Page.goto should not throw an error for a 500 response with an empty body (navigation.spec.js) | expected PASS
Comment 40•8 months ago
|
||
(In reply to Pulsebot from comment #27)
Backout by agoloman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/49ba5bf01339
Backed out 3 changesets for causing multiple failures. CLOSED TREE
Perfherder has detected a browsertime performance change from push 49ba5bf01339012469f5bb95aa4f1108f1dbe16f.
Improvements:
Ratio | Test | Platform | Options | Absolute values (old vs new) | Performance Profiles |
---|---|---|---|---|---|
4% | linkedin LastVisualChange | macosx1015-64-shippable-qr | cold fission webrender | 2,306.98 -> 2,207.65 | Before/After |
3% | linkedin SpeedIndex | macosx1015-64-shippable-qr | fission warm webrender | 604.34 -> 585.15 | Before/After |
Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.
If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.
You can run these tests on try with ./mach try perf --alert 43168
For more information on performance sheriffing please see our FAQ.
Updated•8 months ago
|
Assignee | ||
Updated•8 months ago
|
Assignee | ||
Comment 41•8 months ago
|
||
Planning to first land the response status code patch since it is relatively less risky on regressions, then let's land the blank page patch here.
Comment 42•8 months ago
|
||
Comment on attachment 9426931 [details]
Bug 1325876 - Expose responseStatus and responseStatusText for about:neterror r=#necko,manuel
Revision D223417 was moved to bug 1938947. Setting attachment 9426931 [details] to obsolete.
Comment 43•8 months ago
|
||
(In reply to agoloman from comment #30)
Backed out for causing multiple bc failures.
Perfherder has detected a browsertime performance change from push 6712afee953a1d20ee8124093979a03daf112ca4.
Improvements:
Ratio | Test | Platform | Options | Absolute values (old vs new) | Performance Profiles |
---|---|---|---|---|---|
5% | linkedin PerceptualSpeedIndex | macosx1015-64-shippable-qr | fission warm webrender | 514.02 -> 486.53 | Before/After |
4% | linkedin LastVisualChange | macosx1015-64-shippable-qr | fission warm webrender | 1,637.37 -> 1,570.43 | Before/After |
Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.
If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.
You can run these tests on try with ./mach try perf --alert 43145
For more information on performance sheriffing please see our FAQ.
Comment 44•8 months ago
|
||
(In reply to Pulsebot from comment #27)
Backout by agoloman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/49ba5bf01339
Backed out 3 changesets for causing multiple failures. CLOSED TREE
Perfherder has detected a mozperftest performance change from push 49ba5bf01339012469f5bb95aa4f1108f1dbe16f.
Improvements:
Ratio | Test | Platform | Options | Absolute values (old vs new) |
---|---|---|---|---|
5% | AndroidStartup:fenix cold_main_first_frame.median | android-hw-a55-14-0-aarch64-shippable | 557.54 -> 528.54 | |
5% | AndroidStartup:fenix cold_main_first_frame.median | android-hw-a55-14-0-aarch64-shippable | 555.08 -> 526.83 | |
4% | AndroidStartup:fenix cold_main_first_frame.mean | android-hw-a55-14-0-aarch64-shippable | 551.16 -> 529.07 |
Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.
If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.
You can run these tests on try with ./mach try perf --alert 43216
For more information on performance sheriffing please see our FAQ.
Comment 45•7 months ago
|
||
(In reply to Serban Stanca [:SerbanS] from comment #37)
Backed out for causing multiple failures.
- Push with failures - wpt failures
- Failure Log
- Failure line: TEST-UNEXPECTED-ERROR | /loading/early-hints/404-with-early-hints.h2.window.html | Reached error page: about:neterror?e=httpErrorPage&u=https%3A//web-platform.test%3A9000/loading/early-hints/resources/404-with-early-hints.h2.py%3Fresource-url%3Dhttps%253A%252F%252Fweb-platform.test%253A9000%252Floading%252Fearly-hints%252Fresources%252Fsquare.png%253F7d1545a6-6f8c-4cf4-a293-a78777afebd8&c=UTF-8&d=https%3A//web-platform.test%3A9000/loading/early
- Push with failures - mochitests failures
- Failure Log
- Failure line: TEST-UNEXPECTED-FAIL | browser/components/tabbrowser/test/browser/tabs/browser_link_in_tab_title_and_url_prefilled_normal_page_blank_target.js | Test timed out -
- Push with failures - another wpt failures
- Failure Log
- Failure line: TEST-UNEXPECTED-TIMEOUT | /resource-timing/response-status-code.html | This test validates the response status of resources. 62 - Test timed out
Perfherder has detected a awsy performance change from push e75215ee1dbf77b9f31e2627215a83dbc4e17137.
Regressions:
Ratio | Test | Platform | Options | Absolute values (old vs new) |
---|---|---|---|---|
3% | Images Tabs closed [+30s, forced GC] | windows11-64-2009-shippable-qr | fission tp6 | 1,630,515.33 -> 1,681,829.33 |
As author of one of the patches included in that push, we need your help to address this regression.
Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests. Please follow our guide to handling regression bugs and let us know your plans within 3 business days, or the patch(es) may be backed out in accordance with our regression policy.
If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.
You can run these tests on try with ./mach try perf --alert 43247
For more information on performance sheriffing please see our FAQ.
Comment 46•7 months ago
|
||
Comment 47•7 months ago
|
||
Comment 48•7 months ago
|
||
Backed out for causing Linux related puppeteer failures.
- Backout link
- Push with failures
- Failure Log
- Failure line: TEST-UNEXPECTED-FAIL | navigation Page.goto should not throw an error for a 404 response with an empty body (navigation.spec.js) | expected PASS
Updated•7 months ago
|
Updated•7 months ago
|
Comment 49•7 months ago
|
||
Comment 50•7 months ago
|
||
Comment 51•7 months ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b7a277367308
https://hg.mozilla.org/mozilla-central/rev/38e555f37799
https://hg.mozilla.org/mozilla-central/rev/8f3190a3ce19
Assignee | ||
Comment 52•7 months ago
•
|
||
The patch has been landed, however the error page becomes available only if browser.http.blank_page_with_error_response.enabled
pref is set to false.
Updated•7 months ago
|
Updated•7 months ago
|
Comment 53•7 months ago
|
||
Comment on attachment 9444240 [details]
Bug 1944358 - Set browser.http.blank_page_with_error_response.enabled pref to false by default r=#necko,maltejur
Revision D232429 was moved to bug 1944358. Setting attachment 9444240 [details] to obsolete.
Description
•