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

UnifiedPush: Respond with 404/409/... to Mastodon/etc. instead of 507 based on User-Agent #664

Open
binwiederhier opened this issue Mar 8, 2023 · 11 comments · May be fixed by #842 or #979
Open

UnifiedPush: Respond with 404/409/... to Mastodon/etc. instead of 507 based on User-Agent #664

binwiederhier opened this issue Mar 8, 2023 · 11 comments · May be fixed by #842 or #979
Labels
enhancement New feature or request server Relates to the main binary (server or client) unified-push UnifiedPush feature or bug

Comments

@binwiederhier
Copy link
Owner

Similar to the Matrix Push Gateway change, we should respond to Mastodon/etc. in a way that will make them delete the push subscription.

Mastodon:
https://github.com/mastodon/mastodon/blob/730bb3e211a84a2f30e3e2bbeae3f77149824a68/app/workers/web/push_notification_worker.rb#L35-L46

Common-Proxies:
https://github.com/UnifiedPush/common-proxies/blob/main/gateway/matrix.go#L63

/cc FYI @karmanyaahm @p1gp1g

@binwiederhier binwiederhier added enhancement New feature or request unified-push UnifiedPush feature or bug labels Mar 8, 2023
@p1gp1g
Copy link

p1gp1g commented Mar 8, 2023

Returning 404 for all non existant UnifiedPush requests (where up=1) will follow UnifiedPush spec and Webpush spec and probably avoid many issues :)

RFC 8030 (Webpush)

A push service MAY expire a subscription at any time. If there are
outstanding requests to an expired push message subscription resource
(Section 6) from a user agent or to an expired receipt subscription
resource (Section 6.3) from an application server, this MUST be
signaled by returning a 404 (Not Found) status code
.

UnifiedPush

404

The endpoint does not exist and SHOULD NOT be used anymore. Implementing this is OPTIONAL for both the application server and push server.

@binwiederhier binwiederhier added the server Relates to the main binary (server or client) label Mar 13, 2023
@p1gp1g
Copy link

p1gp1g commented Mar 17, 2023

From the discussion in chat:
Maybe returning 404 after some time (12h ?) for the same reasons we chose to do so for matrix ?

@binwiederhier
Copy link
Owner Author

@karmanyaahm @p1gp1g I am inclined to close this, since nobody has complained. The 5xx rates are still high, but I have just learned to live with it ... Thoughts?

@karmanyaahm
Copy link
Contributor

If no one's having noticable problems, it ought to be fine. Still kinda weird it hasn't normalized down though :/

@Lastorder-DC
Copy link

Lastorder-DC commented Jul 7, 2023

EDITED by @binwiederhier: Removed image containing lots of 507 errors to UP topics. The UP topics are secrets, so I removed the picture.

It keeps happening and increasing.

@binwiederhier
Copy link
Owner Author

binwiederhier commented Jul 7, 2023

@Lastorder-DC please censor the picture. Unified Push topics are secrets.

Edit: I took the liberty of removing the image for you.

@binwiederhier
Copy link
Owner Author

This is happening because your server is pushing messages to topics that do not have any subscribers. They are dead topics.

The alternative to returning 507 would be to let them through, but then you'd get rate limited and banned within a matter of minutes. So you can either live with the errors, or manually purge the topics from Mastodon.

I am also ok with pull requests to handle it specifically for Mastodon if there are suggestions.

@Lastorder-DC
Copy link

@binwiederhier Forgot that. Sorry! (Actually I have sensored them but accidently choose non-sensored version...)

@ShadowJonathan
Copy link

We're seeing a buildup of push messages to ntfy.sh, presumably to UP. Returning a 4XX code so that the queue is deleted would help a lot, since we're continually getting IP banned by the sheer volume of requests backed up.

Also relevant: mastodon/mastodon#26078

@ShadowJonathan
Copy link

I've talked with @karmanyaahm about this issue, and the consensus seems to be that, 12h after deletion, nfty should be returning 410s for a given subscription.

@ShadowJonathan
Copy link

Actually, the .Stale() function will give a better approximation of when to expire.

@ShadowJonathan ShadowJonathan linked a pull request Aug 18, 2023 that will close this issue
@karmanyaahm karmanyaahm linked a pull request Dec 17, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request server Relates to the main binary (server or client) unified-push UnifiedPush feature or bug
Projects
None yet
5 participants