src: fix base64 decoding

[seishun]

Make sure trailing garbage is not treated as a valid Base64 character.

Fixes: #11987

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows [commit guidelines][]

Affected core subsystem(s)

src

cc @bnoordhuis@jorangreef

[seishun]

Do let me know if someone has a better idea for the test.

[addaleax]

I think a good test would be

assert.deepStrictEqual(Buffer.from('w0 ','base64'),Buffer.from('w0','base64'));

and/or checking the actual bytes?

[jorangreef]

Thanks @seishun and @addaleax.

The test is good. Agree on checking the actual bytes:

assert.deepStrictEqual(Buffer.from('w0 ', 'base64'), Buffer.from([195])); 

[jasnell]

Both of the suggested tests are worthwhile...

assert.deepStrictEqual(Buffer.from('w0 ','base64'),Buffer.from('w0','base64'));assert.deepStrictEqual(Buffer.from('w0 ','base64'),Buffer.from([195]));

[targos]

And also from the issue:

assert.strictEqual(Buffer.from('3v6YpUFyK0/hitA2tCDIfdYKw0 g ','base64').toString('base64'),'3v6YpUFyK0/hitA2tCDIfdYKw0g=');

[addaleax]

@seishun Took the liberty of pushing to your branch, hope that’s okay. :) Rebased & tests added from comments.

CI: https://ci.nodejs.org/job/node-test-commit/8812/

I’m going to land this if the CI comes back green. @aqrln Heads up, this is going to conflict with your PRs. ;)

[seishun]

@addaleax I think adding all these tests is redundant. This one conveys intent the best:

assert.deepStrictEqual(Buffer.from('w0 ','base64'),Buffer.from('w0','base64'));

If this test passes but one of the other proposed tests fails, that means base64 encoding is fundamentally broken, which would cause one of the many existing tests to fail.

[addaleax]

@seishun It’s your PR and I’m not going to tell you what to do. 😄 But really, having slightly more tests than necessary won’t hurt anyone…

[addaleax]

@seishun CI is green … I’d say you can merge this with any non-empty subset of the tests here added. I’d prefer all but every assertion here should be sufficient as a regression test for the particular issue this fixes.

[aqrln]

@addaleax thanks for the heads up! It'll conflict with only one of the PRs, and the conflict will be easy to fix, so I'll rebase it as soon as this one lands :)

[seishun]

Landed in 7e0c3ab.

[MylesBorins]

Cherry-picked to v6. Needed manual backport for v4: 83a856af6d

please lmk if you see any issues