Get recent bad certs - Githubissues

ghost commented 10 years ago

@daveschaefer: from short guide on the proper way to get the exceptions dialog we are apparently supposed to retrieve the nsITransportSecurityInfo and hence the SSLState in the onSecurityChange() method of nsIWebProgressListener. Could you please verify if this works as intended! I chose to implement this a hotfix as I might find a cleaner way with all the tabinfo_restructuring anyway (maybe it's already implemented, I don't remember) which makes heavy use of the nsIWebProgressListener functionality.

ghost commented 10 years ago

Bump!

ghost commented 10 years ago

This is superseded by the merge_tabinfo_restructuring. But the questions still apply.

daveschaefer commented 10 years ago

@lambdor By "superceded" do you mean you no longer want me to merge this pull request? If you still want both is there a particular order you need them in? Otherwise I will test this small change first, merge, and then test the larger PR.

ghost commented 10 years ago

By "superceded" do you mean you no longer want me to merge this pull request? If you still want both is there a particular order you need them in?

The large PR takes longer to verify and the getRecentBadCerts fix is implemented the same way. If we want to get a new update out soon you should merge and publish the smaller one. But if we are going to release the tabinfo restructuring later we don't need this changes.

daveschaefer commented 10 years ago

Roger; I'll test and pull this in first so we can release. The AMO process hasn't released the v4.6 update yet, so it will be interesting to see what happens if we send two updates into the queue at once :)

daveschaefer commented 9 years ago

@lambdor Okay, I tried running the code from this PR out of the box, but when I do things like open a new blank tab I get this error: "an internal error occurred: onSecurityChange - TypeError: aWebProgress.SecurityInfo is undefined".

I think perhaps we need some additional checks to make sure we're dealing with security change events of the correct type. I think we should also remove the aBrowser parameter so there is no confusion on what is being passed.

What do you think of adding some error checks like this? https://github.com/daveschaefer/Perspectives/commit/f559888f63b680cb68faac62b60d066f26a27ec9

Here are my test cases:

Can visit https site with CA-approved HTTPS cert, and it works normally
Can visit https site with self-signed cert, and it works normally x firefox security error for the site is automatically overridden, and you don't see the warning page inbetween (this doesn't seem to work for me, even in the main branch. We can address it after your PR is in)
Can open blank tabs open normally and the Perspectives icon remains blue with no error (about:blank)

Do you have any good examples of sites that trigger the "getRecentBadCerts is not a function" error (#143) in v4.6? I have tried several of the reported sites - https://www.pcwebshop.co.uk/ , https://lists.bufferbloat.net/ - but I am not able to reliably trigger the error, which makes it difficult to have confidence in the test results.

ghost commented 9 years ago

On 2014-12-07 22:00, Dave wrote:

@lambdor https://github.com/lambdor Okay, I tried running the code from this PR out of the box, but when I do things like open a new blank tab I get this error: "an internal error occurred: onSecurityChange - TypeError: aWebProgress.SecurityInfo is undefined".

You're right. I get "aWebProgress is null" though. (FF 35a2)

I think perhaps we need some additional checks to make sure we're dealing with security change events of the correct type. I think we should also remove the aBrowser parameter so there is no confusion on what is being passed.

Yes. I just took over the parameter list from the "restructuring" branch which however uses a TabProgressListener (with aBrowser parameters) instead of a plain ProgressListener. Please also keep that in mind when working with the restructured branch!

What do you think of adding some error checks like this? daveschaefer@f559888 https://github.com/daveschaefer/Perspectives/commit/f559888f63b680cb68faac62b60d066f26a27ec9

Sure, if it works. Personally I don't like the multiple-return-points style though but it's a hot-fix anyway.

Do you have any good examples of sites that trigger the "getRecentBadCerts is not a function" error (#143 https://github.com/danwent/Perspectives/issues/143) in v4.6? I have tried several of the reported sites - https://www.pcwebshop.co.uk/ , https://lists.bufferbloat.net/ - but I am not able to reliably trigger the error, which makes it difficult to have confidence in the test results.

From what I remember this error occured with pretty much every site because the function was removed in FF 33 and this PR is the very fix to remove the error :) If you still get this error then it's a bug.

Btw: Do you have any idea why the 4.6 version is still not released on AMO?

Did you already have a chance to look at and test the restructuring branch?

danwent / Perspectives

Get recent bad certs #157