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

When decoding, if JWT payload is not valid it returns null #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dschenkelman
Copy link
Member

No description provided.

@hsablonniere
Copy link

Hey, BTW, there's already a "safe" JSON parse : https://github.com/brianloveswords/node-jws/blob/master/lib/verify-stream.js#L14

I have the same problem so I would really like to see this PR merged.

@dschenkelman
Copy link
Member Author

Wow, had forgotten about this PR! @hsablonniere thanks for the suggestion, updated the code.

Just updated to master in case this could still get in. I know it changes behaviors compared to the old one, but is seems more consistent.

Thoughts?

@hsablonniere
Copy link

Hey Damian,

Long time no see :-)
I don't have any ongoing projects using this. I just took a bit of time to read it and it seems legit.

if (isObject(thing))
return thing;
try { return JSON.parse(thing); }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not supported by the JSON.parse API, see comment here: #86 (comment)

Ref: reviver in https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO only the if (!payload) { return null; } check should be added by this CR, encoding support is addressed in newer CR #86

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

Successfully merging this pull request may close these issues.

3 participants