Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

413 behavior #28

Open
karmanyaahm opened this issue Jun 16, 2022 · 3 comments
Open

413 behavior #28

karmanyaahm opened this issue Jun 16, 2022 · 3 comments

Comments

@karmanyaahm
Copy link
Member

see discussion in #unifiedpush-devs:matrix.org. Will post more details and ideas soon.

@karmanyaahm
Copy link
Member Author

karmanyaahm commented Jun 22, 2022

Current state of things:

  1. ntfy truncates over 4096 bytes.
  2. NextPush and np2p doesn't check for size at all (afaict)
  3. FCM and Gotify (both common-proxies based) return 413 on >4096

My original intent goal when writing the spec was - return 413 if it's too long, and anything under [4096] cannot be too long.

After some discussions, what was actually written was 413 MAY be returned if over 4096. This put the app server in charge of keeping the payload under 4096 by implying unintended consequences.

Going Forward

I now see three main approaches to sort out this disparity.

  1. Push providers MUST return 413 if they are unable to deliver a message fully due to its length. Push Providers MUST support delivery of at least 4096 bytes fully. - This means ntfy will have to change
  2. Push providers MUST return 413 if the message is greater than 4096 bytes and MUST NOT process the message. - I think this is unnecessary overall, might as well do 3. This requires changes to ntfy, NextPush, and np2p.
  3. The push provider MAY return a 413 if the notification payload is too large. The payload length MUST be less than or equal to 4096 bytes. Above that, behavior is undetermined. - This makes returning 413 pointless, but it requires no changes to any distributor.

This is what WebPush (RFC8030) does, quite similar to 1:
Push services might need to limit the size and number of stored push messages to avoid overloading. To limit the size of messages, the push service MAY return a 413 (Payload Too Large) status code [RFC7231] in response to requests that include an entity body that is too large. Push services MUST NOT return a 413 status code in responses to an entity body that is 4096 bytes or less in size.

@p1gp1g
Copy link
Member

p1gp1g commented Jun 22, 2022

The WebPush RFC looks like the 3rd option to me 🤔

We can do the same thing actually:

  • The push provider MAY return a 413 status code if the notification payload is too large. The push provider MUST NOT return a 413 status code in responses to an entity body that is 4096 bytes or less in size.The payload length MUST be less than or equal to 4096 bytes.

@karmanyaahm
Copy link
Member Author

karmanyaahm commented Jun 23, 2022

We can do the same thing actually:

* The push provider MAY return a 413 status code if the notification payload is too large.  The push provider MUST NOT return a 413 status code in responses to an entity body that is 4096 bytes or less in size.The payload length MUST be less than or equal to 4096 bytes.

What you said:
a. The push provider MAY return a 413 status code if the notification payload is too large.
b. The push provider MUST NOT return a 413 status code in responses to an entity body that is 4096 bytes or less in size.
c. The payload length MUST be less than or equal to 4096 bytes.

In my opinion, the following version of that would be the best:
a2. The push provider MUST return a 413 status code if the notification payload is too large. - change MAY to MUST
b. The push provider MUST NOT return a 413 status code in responses to an entity body that is 4096 bytes or less in size. - same
c2. The application server SHOULD send a payload less than or equal to 4096 bytes. - This clearly places the responsibility on the app server. It makes it SHOULD for reasons described below.

Reasoning

This sets a 'recommended' max size. Providers are free to set their size to whatever they want (>=4096). If, for whatever reason, an application could benefit from sending a 5000 byte push notification, this scheme allows it to do so with relative ease. It can send a 5000 byte push, and for providers where there's a 4096 limit, it MUST receive a 413 on a size issue. Then, it can send a smaller push to providers where the limit is 4096.

While I don't see any immediate use for such a capability, keeping options open for app servers can't hurt.

The only downside (in my knowledge) to this idea is that ntfy will have to be changed.
What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants