Closed Bug 1325876 Opened 9 years ago Closed 7 months ago

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)

defect

Tracking

()

RESOLVED FIXED
136 Branch
Tracking Status
firefox50 --- wontfix
firefox51 --- wontfix
firefox52 --- wontfix
firefox53 --- wontfix
firefox136 --- fixed

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)

tracking-fennec: --- → ?
(In reply to Wesley Huang [:wesley_huang] (EPM) (NI me) from comment #1) > Using Chrome to open [1] gets HTTP ERROR 503 error. > > [1] > http://www.theworldin.com/article/12597/martin-luther-church- > splitter?fsrc=scn/tw/wi/bl/ed/ Server replies differently now. Now both chrome and firefox can load page thru the URL.
The website in comment 1 is normal now. Julian gave me this website to test all the http status code: http://ozuma.sakura.ne.jp/httpstatus/ I've checked Chrome could display error message but we only show a blank page.
Flags: needinfo?(sorina.florean)
Summary: URL doesn't load with query string, but does without it → Blank page shown when server returns a 503 error
I get this on desktop nightly too, so I think this might be intentional? Patrick, do we want show our internal load error pages if Content-length=0?
Flags: needinfo?(mcmanus)
I think for the case of 0 bytes of content the front end should indeed show our local error page.. but if there is content then it probably makes sense to render it.
Flags: needinfo?(mcmanus)
So I guess the issue is not fennec specific. I'm changing the product/component.
tracking-fennec: ? → ---
Product: Firefox for Android → Firefox
Hello and sorry for delay here. Tested with Huawei MediaPad M2 (Android 5.1.1) on latest Nightly (54.0a1 from 2017-01-29) and with page http://ozuma.sakura.ne.jp/httpstatus/ from comment 4. Here are the results: - http://ozuma.sakura.ne.jp/httpstatus/503 shows a blank page (same for 101, 200, 201, 202, 203, 206, 207, 208, 304, 305, 306, 400, 401 and the rest of the list) - on Chrome I got the following message that page isn't working (http://ozuma.sakura.ne.jp/httpstatus/) - the first link from description, is working in latest Nightly
Flags: needinfo?(sorina.florean)
Too late for firefox 52, mass-wontfix.
Severity: normal → S3

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.

Type: defect → enhancement
Component: General → DOM: Navigation
Product: Firefox → Core
Summary: Blank page shown when server returns a 503 error → Show internal error page for HTTP responses with error codes (4xx, 5xx) and "Content-Length: 0" instead of blank page
Severity: S3 → --
Type: enhancement → defect
Severity: -- → S3

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.

Priority: -- → P2
Whiteboard: [necko-triaged][necko-priority-queue]
Assignee: nobody → sekim
See Also: → 1890028
See Also: → 1884149

I found this change breaks session history when navigating to a page with HTTP status 500 and Content-Length: 0.

STR:

  1. Open https://emk.name/test/500download.cgi in a new tab with the patched build.
  2. 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).

Component: DOM: Navigation → Networking
Attached image back_button.png (obsolete) —

The updated patch now should have back button enabled unless we do something like ./mach run https://emk.name/test/500download.cgi

Flags: needinfo?(VYV03354)

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:

  1. Open https://bugzilla.mozilla.org/show_bug.cgi?id=1325876 (this bug).
  2. Click a link in this bug to navaigate to https://emk.name/test/500download.cgi.
  3. Click the "500 zero byte response" link.
  4. 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.

Flags: needinfo?(VYV03354) → needinfo?(sekim)

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:

  1. 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()
  1. Build this patch and run ./mach run localhost:8000

  2. 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.

Flags: needinfo?(sekim)
See Also: → 1918167
See Also: → 1763671
Depends on: 1918167
Whiteboard: [necko-triaged][necko-priority-queue] → [necko-triaged][necko-priority-next]
Attachment #9423999 - Attachment is obsolete: true
Blocks: 1763671
See Also: 1763671
Attachment #9428579 - Attachment is obsolete: true
No longer blocks: 1763671
See Also: 1918167
Duplicate of this bug: 1936597
Summary: Show internal error page for HTTP responses with error codes (4xx, 5xx) and "Content-Length: 0" instead of blank page → Show internal error page for HTTP responses with error codes (4xx, 5xx) and "Content-Length: 0" instead of blank page and introduce response status code and text in the error page
Whiteboard: [necko-triaged][necko-priority-next] → [necko-triaged][necko-priority-queue]
Pushed by sekim@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b5a8d01c0651 Show internal error page for HTTP responses with error codes (4xx, 5xx) and Content-Length: 0 instead of blank page r=manuel,necko-reviewers,fluent-reviewers,jesup,bolsson,valentin,kershaw https://hg.mozilla.org/integration/autoland/rev/b3c3a5d3c6b9 Expose responseStatus and responseStatusText for about:neterror r=necko-reviewers,fluent-reviewers,bolsson,kershaw,webidl,saschanaz https://hg.mozilla.org/integration/autoland/rev/4b1363236bcd Add a test for internal error page with error codes (4xx, 5xx) and Content-Length: 0 r=necko-reviewers,jesup,kershaw
Backout by agoloman@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/49ba5bf01339 Backed out 3 changesets for causing multiple failures. CLOSED TREE

Backed out for causing multiple failures.

————————————————————————————————————

————————————————————————————————————

————————————————————————————————————

Flags: needinfo?(sekim)
Pushed by sekim@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c0454be735a8 Show internal error page for HTTP responses with error codes (4xx, 5xx) and Content-Length: 0 instead of blank page r=manuel,necko-reviewers,fluent-reviewers,jesup,bolsson,valentin,kershaw https://hg.mozilla.org/integration/autoland/rev/1092114b16b5 Expose responseStatus and responseStatusText for about:neterror r=necko-reviewers,fluent-reviewers,bolsson,kershaw,webidl,saschanaz https://hg.mozilla.org/integration/autoland/rev/10448d623bd4 Add a test for internal error page with error codes (4xx, 5xx) and Content-Length: 0 r=necko-reviewers,jesup,kershaw
Flags: needinfo?(sekim)

Backed out for causing multiple bc failures.

Flags: needinfo?(sekim)

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

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

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

Flags: needinfo?(sekim)
Pushed by sekim@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0bda3da380ac Show internal error page for HTTP responses with error codes (4xx, 5xx) and Content-Length: 0 instead of blank page r=manuel,necko-reviewers,fluent-reviewers,jesup,bolsson,valentin,kershaw https://hg.mozilla.org/integration/autoland/rev/decca0d92c74 Expose responseStatus and responseStatusText for about:neterror r=necko-reviewers,fluent-reviewers,bolsson,kershaw,webidl,saschanaz https://hg.mozilla.org/integration/autoland/rev/aede2883237a Add a test for internal error page with error codes (4xx, 5xx) and Content-Length: 0 r=necko-reviewers,jesup,kershaw

Backed out for causing bc failures on browser_subdocument_downgrade.js

Backout link

Push with failures

Failure log

This failure is also caused by this bug.

Flags: needinfo?(sekim)
Pushed by sekim@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a4ecb980de87 Show internal error page for HTTP responses with error codes (4xx, 5xx) and Content-Length: 0 instead of blank page r=manuel,necko-reviewers,fluent-reviewers,jesup,bolsson,valentin,kershaw https://hg.mozilla.org/integration/autoland/rev/55446c955dbd Expose responseStatus and responseStatusText for about:neterror r=necko-reviewers,fluent-reviewers,bolsson,kershaw,webidl,saschanaz https://hg.mozilla.org/integration/autoland/rev/8b56e7a25cdd Add a test for internal error page with error codes (4xx, 5xx) and Content-Length: 0 r=necko-reviewers,jesup,kershaw https://hg.mozilla.org/integration/autoland/rev/1c2e847c0632 Add a pref for showing error page for HTTP responses with error codes and Content-Length: 0 r=necko-reviewers,valentin

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


Pushed by sekim@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/014c9c77393a Show internal error page for HTTP responses with error codes (4xx, 5xx) and Content-Length: 0 instead of blank page r=manuel,necko-reviewers,fluent-reviewers,jesup,bolsson,valentin,kershaw https://hg.mozilla.org/integration/autoland/rev/41cc884f84c1 Expose responseStatus and responseStatusText for about:neterror r=necko-reviewers,fluent-reviewers,bolsson,kershaw,webidl,saschanaz https://hg.mozilla.org/integration/autoland/rev/f8add6e2ef93 Add a test for internal error page with error codes (4xx, 5xx) and Content-Length: 0 r=necko-reviewers,jesup,kershaw https://hg.mozilla.org/integration/autoland/rev/b29ef3f14bf7 Add a pref for showing error page for HTTP responses with error codes and Content-Length: 0 r=necko-reviewers,valentin,credential-management-reviewers,tabbrowser-reviewers,dimi,sthompson

Backed out for causing wd fails @ navigation.spec.js

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 
Regressions: 1938904

(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.

Keywords: perf-alert
Flags: needinfo?(sekim)
Summary: Show internal error page for HTTP responses with error codes (4xx, 5xx) and "Content-Length: 0" instead of blank page and introduce response status code and text in the error page → Show internal error page for HTTP responses with error codes (4xx, 5xx) and "Content-Length: 0" instead of blank page
Depends on: 1938947

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 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.

Attachment #9426931 - Attachment is obsolete: true
No longer duplicate of this bug: 1936597
Regressions: 1939018
Regressions: 1939019

(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.

(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.

(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


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.

Pushed by sekim@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/42f5ed6abce5 Show internal error page for HTTP responses with error codes (4xx, 5xx) and Content-Length: 0 instead of blank page r=manuel,necko-reviewers,fluent-reviewers,jesup,bolsson,valentin,kershaw https://hg.mozilla.org/integration/autoland/rev/ba3a433729a6 Add a test for internal error page with error codes (4xx, 5xx) and Content-Length: 0 r=necko-reviewers,jesup,kershaw https://hg.mozilla.org/integration/autoland/rev/2dcbd388c95f Add a pref for showing error page for HTTP responses with error codes and Content-Length: 0 r=necko-reviewers,valentin,credential-management-reviewers,tabbrowser-reviewers,dimi,sthompson
Backout by sstanca@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2edc8f15bb01 Backed out 3 changesets for causing Linux related puppeteer failures. CLOSED TREE

Backed out for causing Linux related puppeteer failures.

Flags: needinfo?(sekim)
Attachment #9444240 - Attachment is obsolete: true
Attachment #9420907 - Attachment description: Bug 1325876 - Show internal error page for HTTP responses with error codes (4xx, 5xx) and Content-Length: 0 instead of blank page r=manuel,#necko → Bug 1325876 - Add a pref for showing internal error page for HTTP responses with error codes (4xx, 5xx) and Content-Length: 0 instead of blank page r=manuel,#necko
Pushed by sekim@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b7a277367308 Add a pref for showing internal error page for HTTP responses with error codes (4xx, 5xx) and Content-Length: 0 instead of blank page r=manuel,necko-reviewers,fluent-reviewers,jesup,bolsson,valentin,kershaw https://hg.mozilla.org/integration/autoland/rev/38e555f37799 Add a test for internal error page with error codes (4xx, 5xx) and Content-Length: 0 r=necko-reviewers,jesup,kershaw
Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → 136 Branch

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.

Flags: needinfo?(sekim)
Blocks: 1944358
Attachment #9444240 - Attachment is obsolete: false
Attachment #9444240 - Attachment description: Bug 1325876 - Add a pref for showing error page for HTTP responses with error codes and Content-Length: 0 r=#necko,maltejur → Bug 1944358 - Set browser.http.blank_page_with_error_response.enabled pref to false by default r=#necko,maltejur

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.

Attachment #9444240 - Attachment is obsolete: true
No longer blocks: 1944358
See Also: → 1944358
Blocks: 1897663
Blocks: 1951765
Blocks: 1945855
No longer blocks: 1951765
See Also: → 1956000
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: