1584685 - Firefox's network error page occasionally says "{$errorMessage}"

Status: VERIFIED FIXED

(Firefox :: Security, defect, P1)

Description

[Daniel Holbert [:dholbert]]

Twice in the past few days, I've gotten a network error page that has this text (notably featuring the variable {$errorMessage} which looks like it's not something that's supposed to be exposed to users in that form):

The page isn’t redirecting properly
An error occurred during a connection to drestructural.com. {$errorMessage}

Error code:
    This problem can sometimes be caused by disabling or refusing to accept cookies.

(In this case I was trying to visit the site http://drestructural.com which is a GoDaddy parked page when it loads properly -- but for me, it was sometimes not loading properly, and we were showing this error page with the literal text {$errorMessage}.

EXPECTED RESULTS:
I should never see "{$errorMessage}" displayed in Firefox UI.

(Unfortunately I don't have concrete STR for this. I've just run into it randomly a few times during normal browsing, often when the network feels flaky.)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screenshot of the error that I saw (notice "$errorMessage" at top rihgt)

Comment 2

[Daniel Holbert [:dholbert]]

I'm using Firefox Nightly 71.0a1 (2019-09-26) BTW. I'm tentatively suspecting that this is a regression because I don't recall seeing this before, and now I've seen it twice in a week. (Both times, I was using FPN, so there may be proxy weirdness involved with triggering the set of conditions required to get this error.)

johannh, I'm guessing/hoping you might know if this bug belongs in a different component & who might be a good person to take a look at it.

Comment 3

[Robert Longson [:longsonr]]

https://dxr.mozilla.org/mozilla-central/source/browser/base/content/aboutNetError.js#395 should be errorMessage not errMsg although that may not be the whole of the fix since that code itself is in a catch block.

Comment 4

[Robert Longson [:longsonr]]

Attachment: Bug 1584685 - correct parameter to ssl-connection-error. r=?

Comment 5

[Robert Longson [:longsonr]]

What does it print if you take my patch?

Comment 6

[Francesco Lodolo [:flod]]

There is already a bug on file (bug 1584585). Duping, since there are already a few bugs duped against that one.

Comment 7

[Francesco Lodolo [:flod]]

Feel free to move the patch there. As far I can tell, that placeable is only used in one string (ssl-connection-error), and the other place seems to have the right variable.
https://searchfox.org/mozilla-central/source/browser/base/content/aboutNetError.js#364

Comment 8

[Daniel Holbert [:dholbert]]

(In reply to Robert Longson [:longsonr] from comment #5)

What does it print if you take my patch?

I don't think I can give a useful reply to that.[1] But if you think the patch is likely correct, I'd encourage you to post it for review on the other bug.

[1] Can't really test the patch because I don't have reliable STR, and I haven't ever reproduced in a fresh profile, and I don't hit this often enough in my normal browsing profile to know how soon I'd expect to see the issue. Also, if the patch fixes the bug and I do hit the right conditions that would've triggered the bug, I don't know that I'd even be aware, because I think I'd just see a normal error page (like I already do most of the time for network errors). :)

Comment 9

[Johann Hofmann [:johannh]]

Reopening to get a fix in

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/abc500b15d3c
correct parameter to ssl-connection-error. r=johannh?

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/abc500b15d3c

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 13

[Johann Hofmann [:johannh]]

Still seeing this in Nightly today :(

(https://docs.pipenv.org/#install-pipenv-today)

Comment 14

[Francesco Lodolo [:flod]]

(In reply to Johann Hofmann [:johannh] from comment #13)

Still seeing this in Nightly today :(

(https://docs.pipenv.org/#install-pipenv-today)

I see a completely different error message (Peer reports it experienced an internal error.), without Fluent variables for the link above.

But I do still see the error here
https://freelibrary.host/?hash=QNxUWN5EGO3kzM1IDNiBTNkRTOmNjNjVDdz9GauknchJnYpxWZlJnZ

Comment 15

[Johann Hofmann [:johannh]]

Attachment: Bug 1584685 - Don't use try..catch to check for non-existent error strings in aboutNetError.js. r=nhnt11

document.l10n.formatValues seems to have changed and now not throw an error but instead
return undefined when no string was found. This broke the implementation which relied
on try..catch to detect non-existent error strings.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2049b3711261
Don't use try..catch to check for non-existent error strings in aboutNetError.js. r=nhnt11

Comment 17

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/2049b3711261

Comment 18

[Francesco Lodolo [:flod]]

Verified on the link in comment 14 and 72.0a1 (2019-10-23) (64 bit)

@Johann
I assume we should uplift this to beta?

Comment 19

[Johann Hofmann [:johannh]]

Comment on attachment 9102882 [details]
Bug 1584685 - Don't use try..catch to check for non-existent error strings in aboutNetError.js. r=nhnt11

Beta/Release Uplift Approval Request

• User impact if declined: Weird error messages with template placeholders
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Access e.g. https://freelibrary.host/?hash=QNxUWN5EGO3kzM1IDNiBTNkRTOmNjNjVDdz9GauknchJnYpxWZlJnZ and check that it shows an error code instead of {$errorMessage}.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): A small front-end patch limited to error pages.
• String changes made/needed: None

Comment 20

[Alexandru Trif, Desktop QA [:atrif]]

Hello! Using STR from comment 19, I reproduced the issue using Firefox 71.0a1 (20190927215713) on Windows 10x64. " {$errorMessage}" is shown instead of error code.
The issue is verified fixed with Firefox 72.0a1 (20191024214023) on Windows 10x64, Ubuntu 18.04 and macOS 10.14. Error code is correctly shown.

Comment 21

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9102882 [details]
Bug 1584685 - Don't use try..catch to check for non-existent error strings in aboutNetError.js. r=nhnt11

Low risk front end patch for error pages, verified by QA in nightly, uplift approved for 71 beta 5, thanks!

Comment 22

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/releases/mozilla-beta/rev/b19acb55364c

Comment 23

[Alexandru Trif, Desktop QA [:atrif]]

Hello! The issue is verified fixed with Firefox 71.0b5 (20191027125310) from comment 22 on Windows 10x64, Ubuntu 18.04 and macOS 10.14.