Closed Bug 987262 Opened 6 years ago Closed 6 years ago

mozilla::pkix::der::Input::AtEnd returns a bool, not a Result (so don't compare the two)

Categories

(Core :: Security: PSM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: keeler, Assigned: keeler)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
https://mxr.mozilla.org/mozilla-central/source/security/pkix/lib/pkixder.h#260
260   if (!nested.AtEnd() != Success) {
261     return Fail(SEC_ERROR_BAD_DER);
262   }

Line 260 should be 'if (!nested.AtEnd()) {'

This is the only instance of this I could find.
Attachment #8395804 - Flags: review?(cviecco)
Attachment #8395804 - Flags: review?(cviecco) → review+
Comment on attachment 8395804 [details] [diff] [review]
patch

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

::: security/pkix/lib/pkixder.h
@@ +256,5 @@
>    if (decoder(nested) != Success) {
>      return Failure;
>    }
>  
> +  if (!nested.AtEnd()) {

This also works:

    return End(input);
(In reply to Brian Smith (:briansmith, was :bsmith; NEEDINFO? for response) from comment #1)
>     return End(input);

I meant End(nested);

I think there should be some compiler warnings that catch this, and warnings-as-errors should have caused the built to fail. I wonder why that didn't happen.
Assignee: nobody → dkeeler
Status: NEW → ASSIGNED
Attached patch patch v2Splinter Review
Ok - I refactored the common checks to use der::End where appropriate.
Attachment #8395804 - Attachment is obsolete: true
Attachment #8395827 - Flags: review?(brian)
Attachment #8395827 - Flags: review?(brian) → review+
https://hg.mozilla.org/mozilla-central/rev/53fcaeb264ba
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.