Closed
Bug 1337348
Opened 8 years ago
Closed 8 years ago
Ensure array buffers and Base64-encoded strings can be passed as app server keys
Categories
(Core :: DOM: Push Subscriptions, defect)
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: collimarco91, Assigned: lina)
Details
(Keywords: dev-doc-complete)
Attachments
(1 file)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/602.3.12 (KHTML, like Gecko) Version/10.0.2 Safari/602.3.12
Steps to reproduce:
I have generated the applicationServerKey (for pushManager.subscribe) as described here:
http://stackoverflow.com/questions/42079185/generate-the-vapid-public-key-in-rails-and-pass-it-to-javascript
Actual results:
pushManager.subscribe throws this exception:
DOMException [InvalidAccessError: "Invalid raw ECDSA P-256 public key."
code: 15
nsresult: 0x8053000f]
Expected results:
It should have accepted the public key, because it is correct.
Chrome accepts it and I could successfully send a notification with VAPID to FCM.
Reporter | ||
Comment 1•8 years ago
|
||
This is the Uint8Array that represents the public key:
[4, 40, 169, 137, 190, 138, 168, 242, 241, 191, 237, 4, 210, 40, 233, 112, 233, 183, 243, 140, 60, 247, 32, 220, 149, 48, 26, 114, 119, 102, 9, 13, 41, 246, 108, 108, 200, 69, 110, 218, 172, 5, 214, 255, 67, 154, 102, 208, 195, 76, 188, 74, 15, 163, 173, 232, 35, 51, 34, 64, 32, 158, 222, 20, 86]
Reporter | ||
Updated•8 years ago
|
OS: Unspecified → Mac OS X
Hardware: Unspecified → x86
Reporter | ||
Comment 2•8 years ago
|
||
Another note: in the docs you don't provide any example about the applicationServerKey and it is still reported as not supported for Firefox in the compatibility table at the bottom of the page (however I think it is actually supported, since I get that exception).
https://developer.mozilla.org/en-US/docs/Web/API/PushManager/subscribe
Assignee | ||
Comment 3•8 years ago
|
||
Hmm, how are you passing the key to `pushManager.subscribe`? I just tried this, and got a subscription with a matching key:
const applicationServerKey = new Uint8Array([
4, 40, 169, 137, 190, 138, 168, 242, 241, 191, 237, 4, 210, 40,
233, 112, 233, 183, 243, 140, 60, 247, 32, 220, 149, 48, 26, 114,
119, 102, 9, 13, 41, 246, 108, 108, 200, 69, 110, 218, 172, 5, 214,
255, 67, 154, 102, 208, 195, 76, 188, 74, 15, 163, 173, 232, 35,
51, 34, 64, 32, 158, 222, 20, 86
]);
registration.pushManager.subscribe({
applicationServerKey: applicationServerKey,
}).then(subscription => {
let actualKey = subscription.options.applicationServerKey;
if (!actualKey) {
throw new Error("Subscription created without key");
}
for (let i = 0; i < actualKey.length; i++) {
if (actualKey[i] != applicationServerKey[i]) {
throw new Error("Mismatched keys");
}
}
return subscription;
}).then(subscription => {
console.log("Created subscription with matching key", subscription);
});
`applicationServerKey` landed in Firefox 48, but it looks like bug 1247685 still has "dev-doc-needed". I'll see if I can find some time to update the docs.
Flags: needinfo?(collimarco91)
Reporter | ||
Comment 4•8 years ago
|
||
Thank you very much!!
I was using:
var bytes = new Uint8Array(len);
registration.pushManager.subscribe({
applicationServerKey: bytes.buffer
});
Now I have removed `.buffer` and it works both on Chrome and Firefox:
var bytes = new Uint8Array(len);
registration.pushManager.subscribe({
applicationServerKey: bytes
});
I don't know if you should also accept `bytes.buffer` or if it worked on Chrome by chance.
Assignee | ||
Comment 5•8 years ago
|
||
(In reply to collimarco91 from comment #4)
> I don't know if you should also accept `bytes.buffer` or if it worked on
> Chrome by chance.
Good catch, you uncovered a bug in Firefox! We should accept array buffers and views, but the array buffer handling was broken for the main-thread `PushManager` implementation.
While I'm here, I'll change `applicationServerKey` so that it accepts a Base64-encoded key, too. This was added in https://github.com/w3c/push-api/pull/227.
Assignee: nobody → kit
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(collimarco91)
Comment hidden (mozreview-request) |
Comment 7•8 years ago
|
||
mozreview-review |
Comment on attachment 8834710 [details]
Bug 1337348 - Ensure array buffers and Base64-encoded strings can be passed as app server keys.
https://reviewboard.mozilla.org/r/110548/#review111906
::: dom/push/Push.js:134
(Diff revision 1)
> + );
> + }
> + } else if (this._window.ArrayBuffer.isView(appServerKey)) {
> + key = appServerKey.buffer;
> + } else {
> + key = appServerKey;
write a comment about what this could be. Probably nothing, right? Then throw.
::: dom/push/PushManager.cpp:583
(Diff revision 1)
> p->MaybeReject(NS_ERROR_DOM_PUSH_ABORT_ERR);
> return p.forget();
> }
>
> nsTArray<uint8_t> appServerKey;
> if (!aOptions.mApplicationServerKey.IsNull()) {
WasPassed
::: dom/push/PushManager.cpp:625
(Diff revision 1)
> + aAppServerKey);
> + } else {
> + MOZ_CRASH("Uninitialized union: expected string, buffer, or view");
> + }
> + if (!ok) {
> + return NS_ERROR_DOM_PUSH_INVALID_KEY_ERR;
what about if:
if (aSource.IsString()) {
...
} else if (aSource.IsArrayBuffer()) {
if (!PushUtil::CopyArrayBufferToArray(aSource.GetAsArrayBuffer(),
aAppServerKey)) {
return NS_ERROR_DOM_PUSH_INVALID_KEY_ERR;
}
} else if (aSource.IsArrayBufferView()) {
if (!PushUtil::CopyArrayBufferViewToArray(aSource.GetAsArrayBufferView(),
aAppServerKey)) {
return NS_ERROR_DOM_PUSH_INVALID_KEY_ERR;
}
} else {
MOZ_CRASH(..);
}
Attachment #8834710 -
Flags: review?(amarchesini) → review+
Assignee | ||
Comment 8•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8834710 [details]
Bug 1337348 - Ensure array buffers and Base64-encoded strings can be passed as app server keys.
https://reviewboard.mozilla.org/r/110548/#review111906
> write a comment about what this could be. Probably nothing, right? Then throw.
Good catch, added a comment that it's an array buffer.
> WasPassed
`mApplicationServerKey` is a `mozilla::dom::Nullable`; looks like it doesn't have a `WasPassed` method.
Comment hidden (mozreview-request) |
Comment 10•8 years ago
|
||
Pushed by kcambridge@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d0174f989cfd
Ensure array buffers and Base64-encoded strings can be passed as app server keys. r=baku
Comment 11•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Assignee | ||
Updated•7 years ago
|
Keywords: dev-doc-needed
Summary: [Web Push API, VAPID] InvalidAccessError: "Invalid raw ECDSA P-256 public key.", but the key works on Chrome → Ensure array buffers and Base64-encoded strings can be passed as app server keys
Comment 12•7 years ago
|
||
I've documented this by adding a sentence to the Subscribe page to clarify what data types are accepted for the key:
https://developer.mozilla.org/en-US/docs/Web/API/PushManager/subscribe#Parameters
I've also added a note to the Fx55 rel notes:
https://developer.mozilla.org/en-US/Firefox/Releases/55#Service_WorkersPush
Let me know if this looks OK, or if you need anything more. Thanks!
Keywords: dev-doc-needed → dev-doc-complete
Assignee | ||
Comment 13•7 years ago
|
||
Thanks, Chris!
You need to log in
before you can comment on or make changes to this bug.
Description
•