497488 - RSS feeds with an invalid certificate fail with a misleading 'url could not be found' error, work if a certificate security exception is added manually

Status: RESOLVED FIXED

(MailNews Core :: Feed Reader, defect)

Description

[Major Péter]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; hu-HU; rv:1.9.1b4) Gecko/20090427 Fedora/3.5-0.20.beta4.fc11 Firefox/3.5b4
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b3pre) Gecko/20090513 Fedora/3.0-2.3.beta2.fc11 Thunderbird/3.0b2

If I would like to subscribe to an RSS feed, which is based on HTTPS and the Cert has problems (sec_error_untrusted_issuer), then a new window will pop up, where are two options:
- view the cert
- cancel

there is no accept cert button.

Reproducible: Always

Steps to Reproduce:
1. RSS Subscriptions
2. Add
3. URL: https://bash.hu/rss for example
Actual Results:  
The described window pops up.

Expected Results:  
There could be an accept cert button.

The workaround could be, that at the Main settings I add the invalid cert manually to the accepted certs list, but this is a big overhead for a regular user I guess. It should be simplier.

Comment 1

[Ludovic Hirlimann [:Usul]]

Confirming Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090609 Shredder/3.0b3pre -

Comment 2

[Major Péter]

What's the state of this bug? I would really love to read RSS's with TB..
Also there is a same (wrong) handling of certifications, when the html e-mail is containing pictures from an invalid-cert HTTPS page.

Comment 3

[Jonathan Protzenko [:protz]]

kaie, is there anything actionable we can do to move this bug forward?

Comment 4

[Mark Banner (:standard8)]

I suspect that bug 493980 may hold the key to this, as its very likely we're doing a similar thing here (i.e. looking at RSS feed in the background and not supplying a window or something like that).

Comment 5

[Kai Engert (:KaiE:)]

A user should be able to manually add an override, if desired.

preferences / advanced / certificates / view / servers / add exception

A dialog opens, as Location enter
  https://bash.hu/rss

Look at the report, and if desired, you may confirm a permanent security exception.

After you did the above, you should be able to connect.

Comment 6

[Ludovic Hirlimann [:Usul]]

(In reply to comment #5)

> After you did the above, you should be able to connect.

confirming that the exception works that way.

Comment 7

[Jan Melcher]

This bug is still present.

It would be even a great improvement to change the error message to something that explains the actual error.

Comment 8

[alta88]

The real problem is the misleading error for a certificate/security exception. It would be easy enough to cut/paste the code in bug 866924 comment 1 but it would be much nicer to use the api if/when it lands.

Comment 10

[alta88]

This patch increases granularity of http error messages, fixes misleading no auth/bad certificate messages, and creates a better ergonomic for user exceptions to overridable security certificate errors.

In the subscribe dialog:
1) immediate feedback when a bad cert feed url is subscribed.
2) for bad certs, display an Add Exception button to open the Security Exception dialog.
3) for existing feed urls, enable the Update button to perform verification to check if the url still works.

Comment 11

[alta88]

Attachment: feedCertificate.patch

Comment 12

[alta88]

Attachment: feedVerify.patch

Comment 13

[alta88]

Attachment: feedVerify.patch

skip item invalidation in non parse/store mode.

Comment 14

[alta88]

Attachment: feedVerify.patch

tweaks for better ui feedback on verify.

Comment 15

[alta88]

Attachment: suiteString.patch

Comment 16

[Magnus Melin [:mkmelin]]

Comment on attachment 8794999 [details] [diff] [review]
feedCertificate.patch

Review of attachment 8794999 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/locales/en-US/chrome/messenger-newsblog/newsblog.properties
@@ +67,5 @@
>  newsblog-networkError=%S could not be found. Please check the name and try again.
>  ## LOCALIZATION NOTE(newsblog-feedNotValid): %S is the feed URL
>  newsblog-feedNotValid=%S is not a valid feed.
> +## LOCALIZATION NOTE(newsblog-badCertError): %S is the feed URL host
> +newsblog-badCertError=%S uses an invalid security certificate; user exception required.

I'd remove the "; user exception required." part

::: mailnews/extensions/newsblog/content/Feed.js
@@ +230,5 @@
>        }
> +      else {
> +        let [errType, errName] = FeedUtils.createTCPErrorFromFailedXHR(request);
> +        FeedUtils.log.info("Feed.onDownloaded: request errType:errName:statusCode - " +
> +                           errType+":"+errName+":"+request.status);

nit: spaces around plusses

Comment 17

[alta88]

Attachment: feedCertificate.patch

updated.

Comment 18

[alta88]

Attachment: suiteString.patch

for suite.

Comment 19

[alta88]

string ping?

Comment 20

[Ian Neal]

Comment on attachment 8800615 [details] [diff] [review]
suiteString.patch

># HG changeset patch
># Parent  109f8303a81d8c21c320fb8e347b5f1cd989136a
>Bug 497488 - RSS feeds with an invalid certificate fail with a misleading 'url could not be found' error, work if a certificate security exception is added manuall
>
>diff --git a/suite/locales/en-US/chrome/mailnews/newsblog/newsblog.properties b/suite/locales/en-US/chrome/mailnews/newsblog/newsblog.properties
>--- a/suite/locales/en-US/chrome/mailnews/newsblog/newsblog.properties
>+++ b/suite/locales/en-US/chrome/mailnews/newsblog/newsblog.properties
>@@ -9,16 +9,18 @@ subscribe-feedAlreadySubscribed=You alre
> subscribe-errorOpeningFile=Could not open the file.
> subscribe-feedAdded=Feed added.
> subscribe-feedUpdated=Feed updated.
> subscribe-feedMoved=Feed subscription moved.
> subscribe-feedCopied=Feed subscription copied.
> subscribe-feedRemoved=Feed unsubscribed.
> subscribe-feedNotValid=The Feed URL is not a valid feed.
> subscribe-networkError=The Feed URL could not be found. Please check the name and try again.
>+subscribe-noAuthError=The Feed URL is not authorized.
>+subscribe-feedVerified=The Feed URL has been verified.
> subscribe-loading=Loading, please waitâ¦
Nit: could the "subscribe-feedVerified" entity be moved to just after the other "subscribe-feed*" entities?
r/a=me with that fixed.

Comment 21

[alta88]

Attachment: suiteString.patch

Comment 22

[alta88]

Attachment: feedVerify.patch

Comment 23

[alta88]

feedCertificate.patch should be applied first, thanks.

Comment 24

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/9fcd32547793baeb1881ed251e796243e1e964e9
https://hg.mozilla.org/comm-central/rev/141676b80c81e2a3daeeac6ad21da35086ae0240
https://hg.mozilla.org/comm-central/rev/324728b47409a90d5feeba83566db016b3da826c

Please don't just copy the commit message from the bug summary. It would also be nice if you could include your full user info in the patch.