disallow empty token names as the argument to nsIPK11TokenDB.findTokenByName

RESOLVED FIXED in Firefox 53

Status

()

Core
Security: PSM
P1
normal
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: keeler, Assigned: keeler)

Tracking

({addon-compat})

Trunk
mozilla53
addon-compat
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox53 fixed)

Details

(Whiteboard: [psm-assigned])

MozReview Requests

()

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

Attachments

(1 attachment)

nsIPK11TokenDB.findTokenByName("") is shorthand for nsIPK11TokenDB.getInternalKeyToken() which is a little silly as it's not all that much shorter and it's a bit unexpected due to not being documented at all. We should just enforce that findTokenByName take a non-empty string and change existing code to call getInternalKeyToken.
Comment hidden (mozreview-request)

Comment 2

5 months ago
mozreview-review
Comment on attachment 8824218 [details]
bug 1328456 - disallow empty token names for nsIPK11TokenDB.findTokenByName

https://reviewboard.mozilla.org/r/102726/#review103486

Looks good.

::: security/manager/pki/resources/content/resetpassword.xul:11
(Diff revision 1)
>  <?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
>  
>  <!DOCTYPE dialog SYSTEM "chrome://pippki/locale/pippki.dtd">
>  
>  <dialog id="reset_password" title="&resetPassword.title;"
>    xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"      

Would be nice to get rid of the trailing whitespace while you're here.

::: security/manager/ssl/nsIPK11TokenDB.idl:38
(Diff revision 1)
>    /*
> -   * Find a token by name
> +   * Find a token by name. Throws NS_ERROR_FAILURE if no token with the given
> +   * name can be found.
> +   * @param tokenName a string identifying the name of the token. Must be
> +   *                  non-empty.
> +   * @return nsIPK11Token a token with the given name

No harm in doing so, but we probably don't need to repeat the return type.
Attachment #8824218 - Flags: review?(cykesiopka.bmo) → review+
Comment hidden (mozreview-request)
(Assignee)

Comment 4

5 months ago
Thanks!

Comment 5

5 months ago
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f1b501863c27
disallow empty token names for nsIPK11TokenDB.findTokenByName r=Cykesiopka
(Assignee)

Comment 6

5 months ago
Try looked good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c0145e52eb6
Also, this could have an add-on compatibility impact. The fix should be fairly obvious, however (just use getInternalKeyToken).
Keywords: addon-compat

Comment 7

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f1b501863c27
Status: NEW → RESOLVED
Last Resolved: 5 months ago
status-firefox53: affected → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.