collimarco91 Reporter
	Description • 7 years ago

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.

	collimarco91 Reporter
	Comment 1 • 7 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]

	collimarco91 Reporter
	Comment 2 • 7 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

	Lina Butler [:lina] Assignee
	Comment 3 • 7 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.

	collimarco91 Reporter
	Comment 4 • 7 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.

	Lina Butler [:lina] Assignee
	Comment 5 • 7 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.

	Andrea Marchesini [:baku]
	Comment 7 • 7 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(..);
}

	Lina Butler [:lina] Assignee
	Comment 8 • 7 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.

	Pulsebot
	Comment 10 • 7 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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 11 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/d0174f989cfd

	Chris Mills [:cmills]
	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!

	Lina Butler [:lina] Assignee
	Comment 13 • 7 years ago

Thanks, Chris!