488480 - Wrong comment about the function hasMatchingOverride() in nsICertOverrideService.idl

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Srirang G Doddihal (Brahmana)]

The function hasMatchingOverride() in nsICertOverrideService.idl has a wrong comment/description. http://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/public/nsICertOverrideService.idl#91

It is an exact replica of the comment for the previous function: http://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/public/nsICertOverrideService.idl#72

Comment 1

[Johnathan Nightingale [:johnath]]

There's a patch in bug 466011 that fixes this. It got lost in the mists of time waiting for review, but should still apply pretty cleanly, since the IDL doesn't change much.

Comment 2

[Kai Engert [:KaiE:]]

reassign bug owner.
mass-update-kaie-20120918

Comment 3

[:Cykesiopka]

Attachment: bug488480_correct-hasMatchingOverride()-doc_by-Johnathan-Nightingale_v1.patch

This patch contains the relevant bits from johnath's patch in Bug 466011, with very trivial changes.

Comment 4

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Comment on attachment 8586562 [details] [diff] [review]
bug488480_correct-hasMatchingOverride()-doc_by-Johnathan-Nightingale_v1.patch

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

LGTM with nits addressed.

::: security/manager/ssl/public/nsICertOverrideService.idl
@@ +63,3 @@
>     *
>     *  @param aHostName The host (punycode) this mapping belongs to
>     *  @param aPort The port this mapping belongs to, if it is -1 then it 

nit: let's remove trailing whitespace while we're here

@@ +63,5 @@
>     *
>     *  @param aHostName The host (punycode) this mapping belongs to
>     *  @param aPort The port this mapping belongs to, if it is -1 then it 
>     *          is internaly treated as 443
>     *  @param aCert The cert that should always be accepted

I would update this description too. Maybe "The certificate this mapping belongs to".

@@ +72,2 @@
>     */
>    boolean hasMatchingOverride(in ACString aHostName, 

nit: trailing whitespace

Comment 5

[:Cykesiopka]

Attachment: bug488480_correct-hasMatchingOverride()-doc_v2.patch

+ Address feedback from Comment 4
+ Fix typos

Comment 6

[:Cykesiopka]

Thanks for the review.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=79e43b4ab4c4

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/755da69810a1

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/755da69810a1