Check argument validity more in nsASN1Tree.cpp

RESOLVED FIXED in Firefox 49

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Cykesiopka, Assigned: Cykesiopka)

Tracking

unspecified
mozilla49
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

(Whiteboard: [psm-assigned])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Many of the functions in nsASN1Tree.cpp don't have checks that ensure their arguments are valid. Checks should be added to ensure callers aren't doing the wrong thing.
(Assignee)

Comment 1

2 years ago
Created attachment 8746944 [details]
MozReview Request: Bug 1268365 - Check argument validity more in nsASN1Tree.cpp. r=jcj

Review commit: https://reviewboard.mozilla.org/r/49635/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/49635/
Attachment #8746944 - Flags: review?(dkeeler)
Comment on attachment 8746944 [details]
MozReview Request: Bug 1268365 - Check argument validity more in nsASN1Tree.cpp. r=jcj

https://reviewboard.mozilla.org/r/49635/#review46533

I'm going to redirect to JC for this review. Also, I wouldn't spend too much effort on fixing up the nsASN1* code or any of the C++ code that implements the certificate manager/viewer, since it should pretty much all be rewritten in JS anyway (which is something I'm hoping will be possible in the nearish future).
Attachment #8746944 - Flags: review?(dkeeler)
Attachment #8746944 - Flags: review?(jjones)
Comment on attachment 8746944 [details]
MozReview Request: Bug 1268365 - Check argument validity more in nsASN1Tree.cpp. r=jcj

https://reviewboard.mozilla.org/r/49635/#review46749

Thank you for doing this cleanup, Cykesiopka! This looks good to me.
Attachment #8746944 - Flags: review?(jjones) → review+
(Assignee)

Comment 4

2 years ago
Comment on attachment 8746944 [details]
MozReview Request: Bug 1268365 - Check argument validity more in nsASN1Tree.cpp. r=jcj

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49635/diff/1-2/
Attachment #8746944 - Attachment description: MozReview Request: Bug 1268365 - Check argument validity more in nsASN1Tree.cpp. → MozReview Request: Bug 1268365 - Check argument validity more in nsASN1Tree.cpp. r=jcj
(Assignee)

Comment 5

2 years ago
(In reply to David Keeler [:keeler] (use needinfo?) from comment #2)
> Also, I wouldn't spend too much
> effort on fixing up the nsASN1* code or any of the C++ code that implements
> the certificate manager/viewer, since it should pretty much all be rewritten
> in JS anyway (which is something I'm hoping will be possible in the nearish
> future).

Oh, cool! Good to know.
(Assignee)

Comment 6

2 years ago
Thanks for the review!

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d231627c8a457e731dcad0a48f3436424bddf9c
(Ignore the Android checkstyle failure - nothing in the patch touches anything that would trigger such failures).
Keywords: checkin-needed

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4f4c042c6d3a
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.