Closed
Bug 1364991
Opened 7 years ago
Closed 7 years ago
WebAuthnTransactionParent.cpp Cast away const here since NSS wants to be able to use non-const functions
Categories
(Core :: DOM: Device Interfaces, enhancement, P2)
Core
DOM: Device Interfaces
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: jcj, Assigned: jcj)
References
(Blocks 1 open bug)
Details
(Keywords: good-first-bug)
Attachments
(1 file, 1 obsolete file)
Reported by Axel: In https://dxr.mozilla.org/mozilla-central/source/dom/webauthn/WebAuthnTransactionParent.cpp#17 We cast away constness unnecessarily - this is only needed for the soft-token's interactions with NSS, so we should remain const here and move the cast down.
Comment 1•7 years ago
|
||
:jc, feel free to adjust the priority best fitting your plan. :)
Priority: -- → P3
Assignee | ||
Comment 2•7 years ago
|
||
I'll move it up to P2 - this is also a good first bug, but I imagine we'll just fix it while doing Bug 1332681.
Keywords: good-first-bug
Priority: P3 → P2
Comment 3•7 years ago
|
||
./mach is happy with this patch
Assignee | ||
Updated•7 years ago
|
Attachment #8869831 -
Flags: review?(jjones)
Assignee | ||
Comment 4•7 years ago
|
||
Comment on attachment 8869831 [details] [diff] [review] const_cast.patch Review of attachment 8869831 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, Axel. The mochitests were happy with it as well. The only thing I'm changing is the const_cast<unsigned char*> instances to const_cast<uint8_t*> to match the others. I've gone ahead with that and pushed the patch to Try here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4d2bfac5e2377e6f8d9134f6d9165fd69c4d6c2 I'm also going to re-open this review in MozReview and give it to qdot for his take. ::: dom/webauthn/U2FSoftTokenManager.cpp @@ +777,4 @@ > > // Decode the key handle > UniqueSECKEYPrivateKey privKey = PrivateKeyFromKeyHandle(slot, mWrappingKey, > + const_cast<unsigned char*>(aKeyHandle.Elements()), These .Elements() calls should all be casting to uint8_t.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8869831 -
Attachment is obsolete: true
Attachment #8869831 -
Flags: review?(jjones)
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8870219 [details] Bug 1364991 - Make U2FTokenManager use const where possible https://reviewboard.mozilla.org/r/141674/#review145314
Attachment #8870219 -
Flags: review?(kyle) → review+
Assignee | ||
Comment 7•7 years ago
|
||
Thanks for the review, :qdot, and thanks for the patch, Axel. Try run came back clean: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4d2bfac5e2377e6f8d9134f6d9165fd69c4d6c2 Marking checkin-needed.
Keywords: checkin-needed
Pushed by ihsiao@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0300ea496f8b Make U2FTokenManager use const where possible r=qdot
Keywords: checkin-needed
Comment 9•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0300ea496f8b
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•6 years ago
|
Assignee: nobody → jjones
You need to log in
before you can comment on or make changes to this bug.
Description
•