2023800 - New certificate error pages contain misleading information on http errors

Status: RESOLVED FIXED

(Firefox :: Security, defect)

Description

[Manuel Bucher [:manuel]]

Attachment: http-error.png

When connecting to a site that returns a http error, the new error page states

Looks like there’s a problem with this site

Nightly can’t connect to the server at gitlab.gnome.org
What can you do about it?

The site could be temporarily unavailable or too busy. Try again in a few moments.

This is plainly wrong. We connected to the server. The server has returned an error code. We should show the error code to the user. This is most noteable for the most common (or most well-known to users) error code 404. (e.g. see https://lab.neon.rocks/status/404). But true for all the other erros to (you can change the number to other 4xx, 5xx in the test url I've provided).

The previous error was:

Looks like there’s a problem with this site

https://gitlab.gnome.org/ sent back an error.

Error code: 406 Not Acceptable

• Check to make sure you’ve typed the website address correctly.

Mdn article about error pages: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status#client_error_responses

Comment 1

[Manuel Bucher [:manuel]]

@jack: Could you please take a look at this one too?

Comment 2

[Jack Brown]

The two error configs affected are httpErrorPage (4xx) and serverError (5xx), both in toolkit/content/errors/net-errors.mjs. Both incorrectly use fp-neterror-offline-intro ("can't connect to the server") as their intro content.

We should restore the more accurate messaging by introducing a new intro string that can accept and show the HTTP status code and use it as the intro text in those two error configs.

Comment 3

[Jack Brown]

Attachment: Bug 2023800 - Show accurate server error message and status code on HTTP error pages - r=niklas

Comment 4

[Jack Brown]

Attachment: HTTP Error 404 Not Found, After Changes

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/1b63ed2a3bf6
https://hg.mozilla.org/integration/autoland/rev/e7fef12d87aa
Show accurate server error message and status code on HTTP error pages - r=niklas,fluent-reviewers,flod,bolsson

Comment 6

[Serban Stanca [:SerbanS]]

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

Comment 7

[Mathew Hodson]

Last good revision: 3ace307f44a058fd8f0836bf7c9252062f861acd
First bad revision: 2b825e14f742a113f46d8ec9c6aecdeee35087cb
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=3ace307f44a058fd8f0836bf7c9252062f861acd&tochange=2b825e14f742a113f46d8ec9c6aecdeee35087cb

Comment 8

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

Set release status flags based on info from the regressing bug 2009206

Comment 9

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

The patch landed in nightly and beta is affected.
:jbrown, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox150 to wontfix.

For more information, please visit BugBot documentation.

Comment 10

[Mathew Hodson]

Attachment: Bug 2023800 - Show accurate server error message and status code on HTTP error pages

Co-authored-by: Mathew Hodson <[e-mail]>

Original Revision: https://phabricator.services.mozilla.com/D288540

Comment 11

[Mathew Hodson]

Attachment: Bug 2023800 - Show accurate server error message and status code on HTTP error pages

Co-authored-by: Mathew Hodson <[e-mail]>

Original Revision: https://phabricator.services.mozilla.com/D288540

Comment 12

[Mathew Hodson]

@jbrown I think it would be good if this fix could be included in the point release 149.0.1. I tried to help out be creating the uplift revisions for beta and release.

On release 149, there can only be one item in the "What can you do about it?" section because the list whatCanYouDoItems was only added in bug 2019611 for Firefox 150.

For both release and beta, I added a Fluent migration script because my understanding is that we cannot uplift a new string. The migration copies the existing strings used by the old error pages. I hope that works for the l10n team. I tested it with .\mach fluent-migration-test .\python\l10n\fluent_migrations\bug_2023800_http_error_code.py

Comment 13

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined/Reason for urgency: Users on Beta will continue to see a generic error message on HTTP error pages instead of the specific HTTP status code and status text returned by the server.
• Code covered by automated testing?: yes
• Fix verified in Nightly?: yes
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: The change is limited to front-end JS and FTL string files with no C++/Rust modifications. A fluent migration script is included to handle the new string for existing locales.
• String changes made/needed?: Yes. New FTL string fp-neterror-http-error-intro-2 added to toolkit/locales/en-US/toolkit/neterror/netError.ftl. A fluent migration script (python/l10n/fluent_migrations/bug_2023800_http_error_code.py) is included.
• Is Android affected?: no

Comment 14

[Phabricator Automation]

firefox-release Uplift Approval Request

• User impact if declined/Reason for urgency: Users will continue to see a generic error message on HTTP error pages instead of the specific HTTP status code and status text returned by the server.
• Code covered by automated testing?: yes
• Fix verified in Nightly?: yes
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: The change is limited to front-end JS and FTL string files with no C++/Rust modifications. A fluent migration script is included to handle the new string for existing locales.
• String changes made/needed?: Yes. New FTL string fp-neterror-http-error-intro-2 added to toolkit/locales/en-US/toolkit/neterror/netError.ftl. A fluent migration script (python/l10n/fluent_migrations/bug_2023800_http_error_code.py) is included.
• Is Android affected?: no

Comment 15

[Jack Brown]

Thanks Mathew! I added some uplift request forms to those patches for beta and release.

Comment 16

[Dianna Smith [:diannaS]]

pinging l10n team as this uplift requires string changes

Comment 17

[Bryan Olsson [:bolsson]]

Concatenating strings in the way proposed in the migration will introduce errors for certain languages that don't rely on spaces as separators, and also might cause some problems in RTL languages.

Instead of concatenating, can we use the two strings separately and display them separated by a new line. E.g. :

example.com sent back an error.
Error code: 406 Not Acceptable

If a migration is necessary, then the migration would be a straightforward 1:1 for the two strings. Or we could avoid a migration if we called the old strings directly (if that's an option).

Comment 18

[Mathew Hodson]

(In reply to Bryan Olsson [:bolsson] from comment #17)

Instead of concatenating, can we use the two strings separately and display them separated by a new line. E.g. :

example.com sent back an error.
Error code: 406 Not Acceptable

If a migration is necessary, then the migration would be a straightforward 1:1 for the two strings. Or we could avoid a migration if we called the old strings directly (if that's an option).

I modified the template so that the two strings are each in a separate <p> element. One of the strings is only in the "appstrings.properties" file, so the migration is still there to copy it to the Fluent file.

Comment 19

[Mathew Hodson]

@jbrown an alternative thing I tried was setting introContent to an array with the two IDs like in ssl-errors.mjs. That didn't work because then the two strings are shown together with no space in between, so it looked like "sent back an error.Error code:" all on one line.

Comment 20

[Bryan Olsson [:bolsson]]

I'm okay with uplifting if we use the updated migration Mathew Hodson kindly provided.

Comment 21

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/457ae23d0c1a
https://hg.mozilla.org/releases/mozilla-beta/rev/d7d4be553bf0

Comment 22

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/c1784f14f827
https://hg.mozilla.org/releases/mozilla-release/rev/19e223bc0aaf

Comment 23

[Bryan Olsson [:bolsson]]

I saw we ran an update on changesets for the dot release, but this landed on Friday and that was the earliest I could run the migration. Can we update the changesets again for release to include the migrated strings for this patch?

Comment 24

[Dianna Smith [:diannaS]]

Oh no! yea we updated a changeset and took the tip of beta ( but that was for a different uplift so I guess this one wasn't taken into consideration). 150 rolls out in RC next week and live on the 21st, would it be an issue if we just wait for 150? We dont have a planned dot release for 149 and it is already built and going out tomorrow.

Comment 25

[Bryan Olsson [:bolsson]]

Ok, it sounds like there's not really a choice here since it's not breaking things, but we'll have one string that's unlocalized in all locales for the rest of 149.