Use of global state in latest version breaks thread safety of JWT.decode

[dklotz]

The commit aea6da4 by @ab320012 switches the implementation of JWT.decode from something completely functional (i.e. where all arguments are passed around as parameters to all called methods) to using lots of module variables (@jwt, @key etc.) which introduce global state that gets shared between all threads. This introduces more or less subtle errors when used in a multi-threaded environment (e.g. puma), since two calls to JWT.decode can overwrite each others key, headers, payload etc. when a context switch happens in the middle of JWT.decode (e.g. after Decode.new(...) but before verify_signature.

Since this all sounds pretty theoretical so far, I've create a simple reproducer that manages to reproduce this bug every time. It basically just calls JWT.decode(JWT.encode(...)) from several threads in parallel and introduces some artificial delay to increase the likelihood of a problem.

[ab320012]

Good catch. Maybe we can create a new object inside the decode method that gets instantiated per call, that way each call will only modify the state of this new object instead of global state.

[dklotz]

Without being really familiar with the codebase: Wouldn't JWT::Decode be a good candidate for this? It already exists, is already used in the same place and the JWT.encode method already basically just wraps JWT::Encode, which is a good pattern to follow, IMHO

[excpt]

Hi @dklotz,

Thanks for the detailed explanation of the issue.

@ab320012 Thanks again for your contribution!