Closed Bug 985201 Opened 6 years ago Closed 6 years ago

rename insanity::pkix to mozilla::pkix

Categories

(Core :: Security: PSM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: keeler, Assigned: keeler)

References

Details

(Keywords: dev-doc-needed)

Attachments

(1 file, 1 obsolete file)

My understanding is we've agreed to rename insanity::pkix to mozilla::pkix. To limit confusion, we should to do the following as soon as possible:

1. Replace all uses of insanity::pkix (and insanity::der, insanity::test, etc.) in the code with mozilla::pkix (and mozilla::der, etc.)
2. Rename security/insanity to security/mozillapkix (I'm not sure about this one, but naming the directory security/mozilla seems wrong). Same with security/insanity/include/insanity, I suppose.
3. Use mozilla::pkix instead of insanity::pkix in any documentation
4. Use mozilla::pkix instead of insanity::pkix in any open bugs (and closed bugs? I'm not sure that would be worth it)

Anything else I'm forgetting?
(In reply to David Keeler (:keeler) from comment #0)
> 1. Replace all uses of insanity::pkix (and insanity::der, insanity::test,
> etc.) in the code with mozilla::pkix (and mozilla::der, etc.)

Some things are in the insanity namespace. Those will need to go into some other namespace, not "mozilla". I think it is fine to put them all in mozilla::pkix.

> 2. Rename security/insanity to security/mozillapkix (I'm not sure about this
> one, but naming the directory security/mozilla seems wrong). Same with
> security/insanity/include/insanity, I suppose.

I would suggest security/pkix instead of security/mozillapkix and security/pkix/include/pkix.

> 3. Use mozilla::pkix instead of insanity::pkix in any documentation
> 4. Use mozilla::pkix instead of insanity::pkix in any open bugs (and closed
> bugs? I'm not sure that would be worth it)

I agree that we don't need to go back and touch any closed bugs.
Attached patch patch (obsolete) — Splinter Review
This consists of a lot of changes, but they're fairly mechanical:

#include "insanity/whatever.h" -> #include "pkix/whatever.h"
insanity::pkix -> mozilla::pkix
insanity::der -> mozilla::pkix::der
insanity::test -> mozilla::pkix::test
useInsanity -> useMozillaPKIX (in xpcshell tests)
use_insanity_verification -> use_mozillapkix_verification (pref name)
CertVerifier::insanity -> CertVerifier::mozillapkix
CertVerifier::InsanityVerifyCert -> CertVerifier::MozillaPKIXVerifyCert
SSL_(SUCCESFUL|FAILED)_CERT_VALIDATION_TIME_INSANITY -> ...TIME_MOZILLAPKIX (telemetry - do we want to do something different/special here?)

file paths:
security/insanity/include/insanity -> security/insanity/include/pkix
security/insanity -> security/pkix
Assignee: nobody → dkeeler
Status: NEW → ASSIGNED
Attachment #8393656 - Flags: review?(brian)
Comment on attachment 8393656 [details] [diff] [review]
patch

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

I did not check for missing stuff. r+

::: security/manager/ssl/tests/unit/tlsserver/lib/OCSPCommon.cpp
@@ +12,5 @@
>  #include "secerr.h"
>  
>  using namespace mozilla;
>  using namespace mozilla::test;
> +using namespace mozilla::pkix::test;

This file changes, but I am ok with this change.
Attachment #8393656 - Flags: review?(cviecco) → review+
Comment on attachment 8393656 [details] [diff] [review]
patch

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

::: security/pkix/include/pkix/bind.h
@@ +29,5 @@
>  #ifdef _MSC_VER
>  #pragma warning(default:4275)
>  #endif
>  
> +namespace mozilla {

Please move this into mozilla::pkix.

This way, there will be no dependencies from namespace mozilla::pkix -> namespace mozilla and no collisions with any std::bind polyfill that may get added to MFBT.

::: security/pkix/lib/pkixbind.cpp
@@ +21,2 @@
>  
> +namespace mozilla {

Ditto here: namespace mozilla { namespace pkix {
Attachment #8393656 - Flags: review?(brian) → review+
Attached patch patch v1.1Splinter Review
Awesome - thanks. Addressed comments, carrying over r+.
https://tbpl.mozilla.org/?tree=Try&rev=619b5258da40
Attachment #8393656 - Attachment is obsolete: true
Attachment #8394326 - Flags: review+
Comment on attachment 8394326 [details] [diff] [review]
patch v1.1

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

::: security/pkix/lib/pkixbind.cpp
@@ +23,3 @@
>  
>  Placeholder1 _1;
>    

d'oh... I may as well fix this whitespace issue.
https://hg.mozilla.org/mozilla-central/rev/04ea38d3515f
https://hg.mozilla.org/mozilla-central/rev/3f80f6fa65b6
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Keywords: dev-doc-needed
Depends on: 1001825
You need to log in before you can comment on or make changes to this bug.