jwt-decode icon indicating copy to clipboard operation
jwt-decode copied to clipboard

Two additional tests?

Open mb21 opened this issue 3 years ago • 1 comments

I was poking around at the source, wondering what this library does differently than a popular StackOverflow answer.

In base64_url_decode.js, I found just two additions this library has:

if (code.length < 2) {
    code = "0" + code;
}
  1. the switch (output.length % 4) { block

However, when I delete those two blocks, the test suite still passes.

Suggestion: add a test for each one of those blocks.

Thanks for considering!

mb21 avatar Jun 16 '22 13:06 mb21

  1. seems to be coming from https://github.com/auth0/jwt-decode/issues/7#issuecomment-169689392 But I cannot see this case happening: the only way code will be a single-digit string is if p is a character with a really low charCodeAt value – the lowest things I see in ASCII tables are things like tab, newline and space, but those all come out as empty string when put into atob.

  2. is adding some padding for some reason?

mb21 avatar Jun 16 '22 13:06 mb21

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you have not received a response for our team (apologies for the delay) and this is still a blocker, please reply with additional information or just a ping. Thank you for your contribution! 🙇‍♂️

stale[bot] avatar Sep 21 '22 05:09 stale[bot]

Thanks for reaching out, and sorry for the delayed reply.

Having looked at it, I added two tests to cover for the padding part, which we believe can be needed but most likely isn't in the majority of cases.

Regarding the code.length < 2 part, we are actualy doing exactly the same as the link on StackOverflow is doing, just with a slightly different approach.

However, those characters are most likely not going to be found in a JWT, but are mostly there for a more general base64 encoding function. To test this, we would need to test our base64encoding function directly. Our tests focus on the JWT part of the decoding, so I would prefer to leave those as it is for now.

Thanks again for reaching out!

frederikprijck avatar Oct 05 '22 11:10 frederikprijck

Thanks!

mb21 avatar Oct 06 '22 12:10 mb21