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

'DecodeError'will replace 'ExpiredSignature' #260

Closed
cannotcodeanymore opened this issue Mar 26, 2018 · 2 comments
Closed

'DecodeError'will replace 'ExpiredSignature' #260

cannotcodeanymore opened this issue Mar 26, 2018 · 2 comments

Comments

@cannotcodeanymore
Copy link

if you catch DecodeError, you will not get ExpiredSignature.

@excpt
Copy link
Member

excpt commented Mar 27, 2018

Hey @ctc1991,

thanks for reporting.

Can you provide sample code to reproduce this issue, please?

@excpt excpt closed this as completed Jul 10, 2018
@standsleeping
Copy link

Hi there @excpt and @ctc1991. I similarly encountered this, and I believe this behavior is attributable to DecodeError being the parent error class of several decoding-related error classes, one of which happens to be ExpiredSignature.

So what this means is that if the code rescues DecodeError ahead of the others, the rescue block will always catch on DecodeError first.

Run this example:

DecodeError = Class.new(StandardError)
ExpiredSignature   = Class.new(DecodeError)

begin
  raise ExpiredSignature.new
rescue DecodeError => exception
  puts "#{exception.class} was raised, rescued in DecodeError"
rescue ExpiredSignature => exception
  puts "#{exception.class} was raised, rescued in ExpiredSignature"
end

This code outputs:

A ExpiredSignature was raised, rescued in DecodeError

Now, reverse the order of those rescue blocks, and the result becomes:

ExpiredSignature was raised, rescued in ExpiredSignature

Which is what I had originally intended when I laid out my first rescue chain.

This behavior can be avoided by making sure the code rescues DecodeError as a last/final measure in the rescue chain. That being said, the reason I happened to have rescued it as the first exception in my code was because it's the error that is raised when no token is present - which in my use case is most common.

By the way, @excpt if you think this is worth looking into, I'd be happy to help on making a change. We could either replace DecodeError as the parent to all of these with a new class (or just use StandardError for all of them), or we could create new specific error classes in the 4-or-so places where DecodeError is used explicitly and not as a parent class:

  • 'Nil JSON web token'
  • 'Not enough or too many segments'
  • 'No verification key available'
  • 'Invalid segment encoding'

Let me know! Thanks.

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

3 participants