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.

Sign up for GitHub

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

Jump to bottom

How to handle Net::HTTPBadRequest 400 InvalidTokenFormat? #52

Open
Tracked by #1
NARKOZ opened this issue Jun 27, 2018 · 5 comments
Open
Tracked by #1

How to handle Net::HTTPBadRequest 400 InvalidTokenFormat? #52

NARKOZ opened this issue Jun 27, 2018 · 5 comments

Comments

@NARKOZ
Copy link

NARKOZ commented Jun 27, 2018

From time to time we're getting this error:

vendor/bundle/ruby/2.4.0/gems/webpush-0.3.4/lib/webpush/request.rb:39:in `perform': host: fcm.googleapis.com, #<Net::HTTPBadRequest 400 InvalidTokenFormat readbody=true>
body:
<HTML>
<HEAD>
<TITLE>InvalidTokenFormat</TITLE>
</HEAD>
<BODY BGCOLOR="#FFFFFF" TEXT="#000000">
<H1>InvalidTokenFormat</H1>
<H2>Error 400</H2>
</BODY>
</HTML>
 (Webpush::ResponseError)
    from vendor/bundle/ruby/2.4.0/gems/webpush-0.3.4/lib/webpush.rb:42:in `payload_send'

Any reason why this isn't handled as Webpush::InvalidSubscription by the gem?
@stevenharman
Copy link
Contributor

Seems like this could be added pretty easily:

webpush/lib/webpush/request.rb

Lines 29 to 40 in f380b76

if resp.is_a?(Net::HTTPGone) || #Firefox unsubscribed response
(resp.is_a?(Net::HTTPBadRequest) && resp.message == "UnauthorizedRegistration") #Chrome unsubscribed response
raise InvalidSubscription.new(resp, uri.host)
elsif resp.is_a?(Net::HTTPNotFound) # 404
raise ExpiredSubscription.new(resp, uri.host)
elsif resp.is_a?(Net::HTTPRequestEntityTooLarge) # 413
raise PayloadTooLarge.new(resp, uri.host)
elsif resp.is_a?(Net::HTTPTooManyRequests) # 429, try again later!
raise TooManyRequests.new(resp, uri.host)
elsif !resp.is_a?(Net::HTTPSuccess) # unknown/unhandled response error
raise ResponseError.new(resp, uri.host)
end

Maybe it was not a known error/response code before now?

@collimarco
Copy link
Contributor

Does InvalidTokenFormat refers to the token in the endpoint URL? Can you try to reproduce that by changing a character inside the endpoint?
If this is the case I agree that Webpush::InvalidSubscription is the right exception.

@Rubioli
Copy link

Rubioli commented Feb 28, 2019 •
edited
Loading

I did change some parts on p256dh & endpoint. On endpoint I was able to reproduce two exceptions: Net::HTTPBadRequest 400 & Webpush::Unauthorized, whereas wrong p256dh gave OpenSSL::PKey::EC::Point::Error for reason & invalid encoding.

@collimarco
Copy link
Contributor

collimarco commented Feb 28, 2019 •
edited
Loading

@Rubioli thanks for the information, however can you be more specific?

  1. Net::HTTPBadRequest 400: this is never raised directly... You will see a Webpush::ResponseError, which is the best that we can do with just the status code (400). In order to raise something specific like Webpush::InvalidSubscription we need also a specific status name... what is the name near 400? A general Bad Request or something specific?
  2. Webpush::Unauthorized is already a specific exception and this cannot be further improved
  3. "OpenSSL::PKey::EC::Point::Error for reason & invalid encoding": can you report the exact / full exceptions that you get in this case? Also with the stack trace

@collimarco
Copy link
Contributor

collimarco commented Mar 17, 2019 •
edited
Loading

I have spent some time investigating these issues, here's my results:

ID Test Push Service Response Ruby Exception
1 Add a letter inside token (Chrome) 400 UnauthorizedRegistration Webpush::Unauthorized
2 Add a letter inside token (Firefox) 404 Webpush::InvalidSubscription
3 Use an invalid URL for the endpoint URI::InvalidURIError
4 Add a letter to p256dh OpenSSL::PKey::EC::Point::Error lib/webpush/encryption.rb:17:in `initialize'
5 Add a letter to auth ArgumentError: invalid base64 lib/webpush.rb:66:in `decode64'
6 Wrong p256dh / auth, but formally valid 201
7 Use a completely wrong token (Chrome) 400 InvalidTokenFormat Webpush::ResponseError

1-2: FCM and Autopush have slightly different behaviors and this library reflects that properly. FCM doesn't even check if the endpoint exists when the VAPID key is not associated to that endpoint, and thus returns an authentication failure. Autopush on the other hand first check if the endpoint exists, and thus returns 404.
3-4-5: This library lets the specific Ruby errors propagate. For me this is ok. However if someone believes that we should wrap those errors into a more general exception (e.g. Webpush::BadSubscription), then we should create a separate issue for that.
6: This is absolutely correct. The client will fail to decrypt payload, but no error is raised because everything seems correct from server-side (you can't do anything to prevent that).
7: I agree that we can catch that specific 400 InvalidTokenFormat and return Webpush::InvalidSubscription. You can reproduce this using the endpoint https://fcm.googleapis.com/fcm/send/abc.

@collimarco collimarco mentioned this issue Apr 10, 2019
Closed
@collimarco collimarco mentioned this issue Nov 26, 2022
Closed
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@stevenharman @collimarco @NARKOZ @Rubioli