Make sure alg parameter value isn't added twice #297
Conversation
|
Hello, @korstiaan! This is your first Pull Request that will be reviewed by Ebert, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information. |
![sourcelevel-bot[bot]](https://avatars.githubusercontent.com/in/597?s=60&v=4){kind=small}
korstiaan
force-pushed
the
alg_header_conflict
branch
from
42821ee
to
e0e2e0e
Compare
lib/jwt/encode.rb
Outdated
| @@ -39,7 +39,7 @@ def encoded_header_and_payload | |||
| end | |||
|
|
|||
| def encode_header | |||
| encode(@headers.merge(ALG_KEY => @algorithm)) | |||
| encode(@headers.transform_keys(&:to_s).merge(ALG_KEY => @algorithm)) | |||
There was a problem hiding this comment.
I think Hash#transform_keys was added to Ruby 2.5.0, so this won't work on older rubies. I think something like what is done here could be applied for the headers also.
Also if a copy of the @headers hash is created the Hash#merge! method could be used to save one object allocation.
There was a problem hiding this comment.
I've moved the stringify'ing (as done in the claims validator) to the constructor, and directly add ALG_KEY to the headers hash instead of .merge
The
algparameter key (ALG_KEY) is a string, and added to the header hash as such ({ "alg" => "foo" }). If it's already in the header hash, but as a symbol ({ alg: "foo"}), it's appended to the this hash ({ alg: "foo", "alg" => "foo"}`) resulting in the end in a different token.We noticed Apple's Mapkit JS complaing about this.
This PR fixes this.