Closed Bug 1584685 Opened 4 months ago Closed 3 months ago

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

Categories

(Firefox :: Security, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected
firefox69 --- unaffected
firefox70 --- unaffected
firefox71 --- verified
firefox72 --- verified

People

(Reporter: dholbert, Assigned: johannh)

References

Details

(Keywords: regression)

Attachments

(3 files)

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

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.

Flags: needinfo?(jhofmann)

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.

What does it print if you take my patch?

Flags: needinfo?(dholbert)

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

Status: NEW → RESOLVED
Closed: 4 months ago
Flags: needinfo?(jhofmann)
Flags: needinfo?(dholbert)
Resolution: --- → DUPLICATE
Duplicate of bug: 1584585

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

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

Reopening to get a fix in

Status: RESOLVED → REOPENED
Component: General → Security
Priority: -- → P1
Resolution: DUPLICATE → ---
Assignee: nobody → longsonr
Status: REOPENED → ASSIGNED
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/abc500b15d3c
correct parameter to ssl-connection-error. r=johannh?
Status: ASSIGNED → RESOLVED
Closed: 4 months ago3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71

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

Keywords: regression
See Also: → 1584585

Still seeing this in Nightly today :(

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

Assignee: longsonr → jhofmann
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED

(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

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.

Pushed by jhofmann@mozilla.com:
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
Status: ASSIGNED → RESOLVED
Closed: 3 months ago3 months ago
Resolution: --- → FIXED

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

@Johann
I assume we should uplift this to beta?

Status: RESOLVED → VERIFIED
Flags: needinfo?(jhofmann)

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
Flags: needinfo?(jhofmann)
Attachment #9102882 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

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

Attachment #9102882 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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

Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.