Improve error messages from serviceWorkerRegistration.pushManager.subscribe() |
|||||
Issue descriptionVersion: 54.0.2824.0 canary OS: OS X What steps will reproduce the problem? (1) Call serviceWorkerRegistration.pushManager.subscribe() with an invalid applicationServerKey. (e.g. array with too many or too few elements.) What is the expected output? Chrome devtools console reports that something is wrong with the key. For example, "InvalidAccessError: Invalid raw ECDSA P-256 public key." (FF error message.) What do you see instead? DOMException: Registration failed - push service error This could refer to an incorrect argument, or it could refer to a problem with the service endpoint.
,
Aug 10 2016
,
Aug 10 2016
,
Sep 23 2016
We still want to allow the flexibility for someone to pass a sender ID, to allow webapps which aren't ready to adopt VAPID to use a non-manifest subscribe(). However, we can add some additional checking to validate that the key is either a valid P-256 key or a numeric sender ID.
,
Oct 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3f9e6105d28fbb930d90e38c23d07a9af9b53dbc commit 3f9e6105d28fbb930d90e38c23d07a9af9b53dbc Author: harkness <harkness@chromium.org> Date: Mon Oct 17 10:34:23 2016 Check the format of an applicationServerKey when used to register a push subscription. The Web Push specificatoion requires applicationServerKeys to be valid VAPID keys, but we have not previous enforced that, which has led to issues where web developers attempt to subscribe with invalid keys. The error eventually returned is not informative. This CL adds code in Blink to do some basic sanity checking that the provided value is syntactically valid. Although the spec only allows p256 keys to be valid, this code also allows the web developer to subscribe with a numeric sender ID as an argument to subscribe() instead of specifying it in the manifest file, but checks that the ID provided is a number. This CL also adds testing for the new syntax checking. BUG= 636022 Review-Url: https://codereview.chromium.org/2411733002 Cr-Commit-Position: refs/heads/master@{#425649} [modify] https://crrev.com/3f9e6105d28fbb930d90e38c23d07a9af9b53dbc/chrome/browser/push_messaging/push_messaging_browsertest.cc [modify] https://crrev.com/3f9e6105d28fbb930d90e38c23d07a9af9b53dbc/chrome/test/data/push_messaging/push_test.js [add] https://crrev.com/3f9e6105d28fbb930d90e38c23d07a9af9b53dbc/third_party/WebKit/LayoutTests/http/tests/push_messaging/application-server-key-format-test.html [modify] https://crrev.com/3f9e6105d28fbb930d90e38c23d07a9af9b53dbc/third_party/WebKit/LayoutTests/http/tests/push_messaging/application-server-key-standard-endpoint.html [modify] https://crrev.com/3f9e6105d28fbb930d90e38c23d07a9af9b53dbc/third_party/WebKit/LayoutTests/http/tests/push_messaging/push-subscription-options.html [add] https://crrev.com/3f9e6105d28fbb930d90e38c23d07a9af9b53dbc/third_party/WebKit/LayoutTests/http/tests/push_messaging/resources/push-constants.js [modify] https://crrev.com/3f9e6105d28fbb930d90e38c23d07a9af9b53dbc/third_party/WebKit/LayoutTests/http/tests/push_messaging/resources/test-helpers.js [modify] https://crrev.com/3f9e6105d28fbb930d90e38c23d07a9af9b53dbc/third_party/WebKit/LayoutTests/http/tests/push_messaging/subscribe-failure-no-manifest-in-service-worker.html [modify] https://crrev.com/3f9e6105d28fbb930d90e38c23d07a9af9b53dbc/third_party/WebKit/Source/modules/push_messaging/PushSubscriptionOptions.cpp
,
Jun 30 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by sheriffbot@chromium.org
, Aug 10 2016