Closed
Bug 1262645
Opened 8 years ago
Closed 8 years ago
Address misc issues with nsGetUserCertChoice()
Categories
(Core :: Security: PSM, defect)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: Cykesiopka, Assigned: Cykesiopka)
References
()
Details
(Whiteboard: [psm-assigned])
Attachments
(1 file)
nsGetUserCertChoice() has various issues: - Returning a failure result when failing to get a pref value instead of more gracefully falling back to a default. - Using an enum instead of a more strongly typed enum class. - Using a pref branch instead of the preferred Preferences.h API. - Manual memory management. - Unnecessary use of pointers. These can/should be fixed.
Assignee | ||
Comment 1•8 years ago
|
||
The follow issues are fixed: - Returning a failure result when failing to get a pref value instead of more gracefully falling back to a default. - Using an enum instead of a more strongly typed enum class. - Using a pref branch instead of the preferred Preferences.h API. - Manual memory management. - Unnecessary use of pointers. Review commit: https://reviewboard.mozilla.org/r/46825/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46825/
Attachment #8741907 -
Flags: review?(dkeeler)
Attachment #8741907 -
Flags: review?(dkeeler) → review+
Comment on attachment 8741907 [details] MozReview Request: Bug 1262645 - Address misc issues with nsGetUserCertChoice(). r=keeler https://reviewboard.mozilla.org/r/46825/#review43467 Cool. ::: security/manager/ssl/nsNSSIOLayer.cpp:69 (Diff revision 1) > key.AppendASCII(":"); > key.AppendInt(port); > } > > -// SSM_UserCertChoice: enum for cert choice info > -typedef enum {ASK, AUTO} SSM_UserCertChoice; > +// Possible behaviors for choosing a cert for client auth. > +enum class UserCertChoice { Seems like this doesn't need to be defined so far away from where it's used, but not a big deal.
Assignee | ||
Comment 3•8 years ago
|
||
https://reviewboard.mozilla.org/r/46825/#review43467 Thanks for the review! > Seems like this doesn't need to be defined so far away from where it's used, but not a big deal. Fair point, moved.
Assignee | ||
Comment 4•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e14422c8c9d8c4d81c303b525ade6d462b3ae98d
Keywords: checkin-needed
Assignee | ||
Comment 5•8 years ago
|
||
Comment on attachment 8741907 [details] MozReview Request: Bug 1262645 - Address misc issues with nsGetUserCertChoice(). r=keeler Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46825/diff/1-2/
Attachment #8741907 -
Attachment description: MozReview Request: Bug 1262645 - Address misc issues with nsGetUserCertChoice(). → MozReview Request: Bug 1262645 - Address misc issues with nsGetUserCertChoice(). r=keeler
Comment 6•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/313b65f1fc525b1f241a86a2da6bbc9a03c64558
Keywords: checkin-needed
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/313b65f1fc52
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•