Closed Bug 391434 Opened 13 years ago Closed 11 years ago

avoid multiple encoding/decoding of PKIX_PL_OID to and from ascii string

Categories

(NSS :: Libraries, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED FIXED
3.12.4

People

(Reporter: alvolkov.bgs, Assigned: alvolkov.bgs)

Details

(Whiteboard: PKIX)

Attachments

(2 files, 3 obsolete files)

libpkix keeps oids as an array of unsigned integers. But this is just a part of the problem. The rest of the problem is that there are multiple places in the code where pkix oid structure is created from ascii string that represents an OID.
Whiteboard: PKIX
Alexei, libpkix keeps OIDs in lots of different forms in different places.
So, WHERE does it keep OIDs as an array of unsigned integers?
struct PKIX_PL_OIDStruct {
        PKIX_UInt32 *components;
        PKIX_UInt32 length;
};

"components" is a pointer the array of PKIX_UInt32 with length "length".
Ah, yes, that's YET ANOTHER form in which libPKIX stores OIDs.
It's an array of integers, so that an oid like 2.6.13.313976.1.2
becomes an array like
   2
   6
   13
   313976
   1
   2
with a length of 7.
Assignee: nobody → alexei.volkov.bugs
Version: 3.12 → trunk
Priority: -- → P3
Unsetting target milestone in unresolved bugs whose targets have passed.
Target Milestone: 3.12 → ---
Attachment #372344 - Flags: review?(nelson)
Comment on attachment 372344 [details] [diff] [review]
Patch v1 - first draft of the req changes

Found miner things that need to be fixed in the next patch.
Attachment #372344 - Flags: review?(nelson) → review-
Attached patch Patch v2 - review changes (obsolete) — Splinter Review
Attachment #372344 - Attachment is obsolete: true
Attachment #372364 - Flags: review?(nelson)
Attachment #372364 - Attachment description: Patch v2 - post review changes → Patch v2 - review changes
Comment on attachment 372364 [details] [diff] [review]
Patch v2 - review changes 

Made a mistake in the patch. Removing the request.
Attachment #372364 - Attachment is obsolete: true
Attachment #372364 - Flags: review?(nelson)
Attachment #372429 - Flags: review?(nelson)
Comment on attachment 372429 [details] [diff] [review]
Patch v3 - address review comments

The patch has extra file in the diff. Impossible to do diff of the diffs. :( Will reattach again.
Attachment #372429 - Attachment is obsolete: true
Attachment #372431 - Flags: review?(nelson)
Attachment #372429 - Flags: review?(nelson)
Attachment #372432 - Flags: review?(nelson)
Comment on attachment 372431 [details] [diff] [review]
Patch v4 - patches files included in patch v1(committed)

r=nelson,
I suggest one minor string change

>+    OD( x509CertificatePoliciesAnyPolicy, SEC_OID_X509_ANY_POLICY,
>+ 	"Certificate Policies AnyPolicy",

I suggest"
 	"Certificate Policy AnyPolicy",
or perhaps
 	"Certificate Policy \"AnyPolicy\"",

>+        CKM_INVALID_MECHANISM, UNSUPPORTED_CERT_EXTENSION ),
> };
Attachment #372431 - Flags: review?(nelson) → review+
Comment on attachment 372432 [details] [diff] [review]
Patch v4 - changes to new files(committed)

r=nelson, after one required string change:


>+ER3(SEC_ERROR_BAD_INFO_ACCESS_METHOD,      		(SEC_ERROR_BASE + 170),
>+"Unknown info access method.")

Change that to

  "Unknown information access method in certificate extension."
Attachment #372432 - Attachment description: Patch v4 - chainges to new files → Patch v4 - changes to new files
Attachment #372432 - Flags: review?(nelson) → review+
Attachment #372432 - Attachment description: Patch v4 - changes to new files → Patch v4 - changes to new files(committed)
Attachment #372431 - Attachment description: Patch v4 - patches files included in patch v1 → Patch v4 - patches files included in patch v1(committed)
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.12.4
You need to log in before you can comment on or make changes to this bug.