Decoding the same string outputs different results between 1.2.8 and 2.0.0

[willdurand]

Hi,

one of our test cases caught what looks like a regression when trying to upgrade to 2.0.0.

Here is a snippet extracted from our app:

base62.setCharacterSet('0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz');

var encodedTimestamp = 'not-auth-data:^&invalid-characters:not-a-signature'.split(':')[1]
base62.decode(encodedTimestamp);

Version 1.2.8 outputs -1.1412490322742814e+34, see: https://runkit.com/embed/t6myhreeij3j

Version 2.0.0 (latest) outputs NaN, see: https://runkit.com/embed/szbz01t0km52

[FND]

Thanks for the test cases, that's helpful.

So to summarize: For invalid inputs, v1.2.8 returns a number whereas v2.0.0 returns NaN. It seems to me that v1.2.8's behavior is misleading and thus a bug, so I'm hesitant to change the behavior in v2.x.

Thus I'd recommend you adjust your test cases to reflect this new, arguably more correct behavior. I had a brief look at the PR linked above, but lack sufficient context to assess the impact for your application - please let me know if that works for you.

(FWIW, if we ever get around to it, v3.0 will probably be more strict, validating inputs to avoid such confusion - see #57.)

[willdurand]

Thus I'd recommend you adjust your test cases to reflect this new, arguably more correct behavior. I had a brief look at the PR linked above, but lack sufficient context to assess the impact for your application - please let me know if that works for you.

That makes sense, I'll update our code.

[willdurand]

@FND could you maybe document this behavior?

[FND]

I'm afraid it appears that behavior for invalid inputs is inconsistent:
master...FND:invalid-inputs

So documenting this isn't straightforward - that's unfortunate, of course, but I'd rather expend energy on fixing the underlying issues for v3.0 (see above).

[willdurand]

So documenting this isn't straightforward - that's unfortunate, of course, but I'd rather expend energy on fixing the underlying issues for v3.0 (see above).

ah, oops. Ok, then hopefully everything will be ok and people will find more info in the issue tracker.

Thanks a lot for your work on this lib!

[FND]

I'm glad it's proving useful. (For my part, I was mostly moving existing code around, building on Andrew's original work.)

Since you're one of the few vocal users: Would migrating to the/a new API with strict input validation be a realistic option for you? From my perspective, that should be pretty straightforward and quick, but I don't wanna presume...
(No promises though; I still don't have the headspace to return to the aforementioned v3.0 efforts.)

[willdurand]

Since you're one of the few vocal users: Would migrating to the/a new API with strict input validation be a realistic option for you? From my perspective, that should be pretty straightforward and quick, but I don't wanna presume...
(No promises though; I still don't have the headspace to return to the aforementioned v3.0 efforts.)

I don't have a strong opinion on this, of course we'd like to keep the current API (or actually the legacy one) because it works well for us. That being said, if there is a new API and some upgrade instructions, then no problem. I did not switch to the new API yet because it requires us to pass the character set to encode or decode. The legacy API had a helper function to set this character set globally. If I had to switch to the current/new API right now, I'd probably mimic this legacy API:

import base62 from 'base62/lib/custom';

const charset = base62.indexCharset(
  '~9876543210ABCDEFGHIJKLMNOPQRSTU$#@%!abcdefghijklmnopqrstuvw-='
);

return {
  decode: (data) => base62.decode(data, charset),
  encode: (data) => base62.encode(data, charset),
};

We use this lib to encode/decode auth tokens with a Django backend (hence the need for a custom character set).

[FND]

That's helpful feedback, thank you!