src: fix decoding base64 with whitespace

[seishun]

max_i should also include the characters that were just read by
base64_decode_group_slow.

Fixes: #13636
Fixes: #13657

Please do review the commit message as well.

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)

buffer

[mscdex]

CI: https://ci.nodejs.org/job/node-test-pull-request/8636/

[seishun]

I've replaced "buffer" with "src" in the commit message since this code isn't used just for the buffer module. Any objections?

[addaleax]

Sorry for being so late – can you also include the patch from #13636?

[seishun]

I'm having reservations about including it here, it seems barely related, and would probably confuse someone looking at the commit.

Besides, that patch isn't a proper fix for the test since null char is a valid byte. The right thing would be to compare the decoded length with the expected length.

[addaleax]

I'm having reservations about including it here, it seems barely related, and would probably confuse someone looking at the commit.

You can totally include it as a separate commit, if that helps. (If you don’t want to include it, fine by me – but we should fix the test in any case.)

Besides, that patch isn't a proper fix for the test since null char is a valid byte.

Right, good point. I’d be okay with accepting that because our expected texts don’t contain null chars, but if you have reservations I can do a PR myself.

[seishun]

because our expected texts don’t contain null chars

Right, they don't, but what if our base64_decode implementation starts erroneously padding its output with nulls? The test should fail then.

I can do a PR myself.

That would be great.

[MylesBorins]

Should this be backported to v6.x?

[seishun]

If it applies cleanly, then yes.

[addaleax]

If not, we should probably still backport the test piece here. (edit: to be clear, that passes on its own on 6.11.1)

[aqrln]

@MylesBorins this PR fixes a bug introduced in #12146, so it depends on whether that patch was backported or not.

[MylesBorins]

opted to not land, please feel free to manually backport the test