Closed Bug 1188957 Opened 5 years ago Closed 3 months ago

Separate mozilla::pkix::Input/Reader from rest of DER parsing

Categories

(NSS :: Libraries, enhancement)

enhancement

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: briansmith, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(1 file)

1. Change the result of Input and Result functions to a new type that is different from mozilla::pkix::Result, so that they can be more easily used outside of DER parsing.

2. Change the result type of Input and Result functions so that it is more convenient (fewer lines of code) to string multiple calls together.

3. Make it so that Input.h does not include any header files other than standard header files.

4. Refactor tests to take these changes into account.
Comment on attachment 8640594 [details] [diff] [review]
Separate mozilla::pkix::Input/Reader from rest of DER parsing

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

Ok - LGTM. Just the one comment.

::: security/pkix/test/gtest/pkixcert_signature_algorithm_tests.cpp
@@ +83,5 @@
>        return Success;
>      }
>      Input issuerCert;
> +    if (issuerCert.Init(issuerDER->data(), issuerDER->length()) != Input::OK) {
> +      return Result::FATAL_ERROR_INVALID_STATE;

In pkixbuild_tests.cpp, similar failures return Result::ERROR_BAD_DER (unless I'm misunderstanding the context). We should probably be consistent one way or another.
Attachment #8640594 - Flags: review?(dkeeler) → review+
This is a bug worth fixing but I don't have the time to finish this, so unassigning myself.
Assignee: brian → nobody
Status: ASSIGNED → NEW
Assignee: nobody → nobody
Severity: normal → --
Type: defect → enhancement
Component: Security: PSM → Libraries
Priority: P5 → --
Product: Core → NSS
QA Contact: jjones
Target Milestone: mozilla42 → ---
Version: Trunk → other
Whiteboard: [psm-cleanup]

Since we moved pkix/input.h into NSS, it has effectively become only an ASN.1 parser anyway. If we end up wanting to use it for other purposes in the future we can reopen this bug.

Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.