w3c / push-api

Push API
https://w3c.github.io/push-api/
Other
145 stars 40 forks source link

User Agents should report accepted content-encoding types. #251

Closed jrconlin closed 7 years ago

jrconlin commented 7 years ago

Recently, a bug filed against a webpush subscription library highlighted a shortcoming.

https://github.com/web-push-libs/web-push-php/issues/48#issuecomment-295416292

Currently, there are two in production encryption encoding types, "aesgcm" and "aes128gcm". The "voice of authority" about what types of accepted content types is the UA. The sorts of allowed encryption is not communicated to the subscription update provider.

I would like to propose that the returned PublishSubscription object options object be modified to include a "encodingtypes" list of allowed ECE encoding types. (e.g. ["aesgcm", "aes128gcm"]) This method would also allow future content types to be relayed. If no "encodingtypes" field is present, then the provider must assume "aesgcm" encoding, to allow for older UAs.

This field would also help indicate "updated" UAs which can take advantage of the newer draft specifications.

(Some additional, albeit incorrectly located, discussion is here.)

beverloo commented 7 years ago

(Google hat)

I support this. I'll echo Martin's comment that the data should not be associated with a PushSubscription instance because it applies to the user agent, as we assume that the push service will simply pass the payload through.

I'm ambivalent about whether a property to indicate whether messages without a payload—which therefore are unauthenticated— should be added. In practice all implementations support this and the specification specifically allows for this (11.2.2.5), so I'm not convinced anyone would actually use it.

(Editor hat)

We're very close to marking v1 as done, as in, this very week. I would be OK including this since it's trivial and has clear benefit, but would like to see support on this from @martinthomson and @ShijunS.

Proposal:

partial interface PushManager {
    static readonly attribute FrozenArray<DOMString> supportedContentEncodings;
};

Since the PushManager is available in documents, as well as service, shared and dedicated workers, it's broadly usable as the following example:

if (PushManager.supportedContentEncodings.includes('aes128gcm')) { .. }

If others think that adding a boolean for indicating whether empty messages are supported has sufficient value we'd add a static boolean. Admittedly it's a cheap addition, but I'm stuck in regards to what to name it. supportsEmptyPayload with some extra language in 11.2.2.5?

ShijunS commented 7 years ago

+1 for having this in v1

martinthomson commented 7 years ago

My sense is that we can add supportedContentEncodings easily, without too much effort, so we should. The boolean gets a little tricky. Let's revisit that if we ever get to the point where supporting empty payloads is at risk. I suspect that we'll find that pulling that feature out will be impossible.

beverloo commented 7 years ago

Great. I'll share a PR tomorrow (Friday) for this and the other remaining v1 issue so that we can wrap that up.