Issue #4128 - Add missing padding and use URL decoder

[travisspencer]

This fixes the issue (#4128) I raised about incorrect decoding of some ID token JWTs that have sections that need their padding added back.

[joakime]

Your commits do not satisfy the Eclipse Legal requirement for a signed ECA. (See details of commit checks)
We are unable to merge without a signed ECA.
See https://github.com/eclipse/jetty.project/blob/jetty-9.4.x/CONTRIBUTING.md#eclipse-contributor-agreement

[lachlan-roberts]

I don't think the case here with === is necessary.

This could be simplified using the example from
https://tools.ietf.org/html/rfc7515#appendix-C

Something like:

static byte[] b64DecodeWithoutPadding(String arg)
{
    String s = arg;
    switch (s.length() % 4) // Pad with trailing '='s
    {
        case 0: break; // No pad chars in this case
        case 2: s += "=="; break; // Two pad chars
        case 3: s += "="; break; // One pad char
        default: throw new IllegalArgumentException("Illegal base64url string!");
    }
    return Base64.getUrlDecoder().decode(s); // Standard base64 decoder
}

[travisspencer]

Ya, that is simpler. Will look into that.

[travisspencer]

@lachlan-roberts , @joakime et a. maybe you can help with this, but it seems the above code is licensed under BSD. See https://trustee.ietf.org/documents/IETF-TLP-5_001.html. So, is it OK to bring this in? Does something need to be done, like an attribution or something?

[lachlan-roberts]

@travisspencer how about we write this in a different way with the same resulting logic.

remainder    action       (4-remainder)%4
-----------------------------------------
0            0 padding        0
1            error            -
2            2 padding        2
3            1 padding        1

private static byte[] padBase64(byte[] unpaddedBase64)
{
    int remainder = unpaddedBase64.length % 4;
    if (remainder == 1)
        throw new IllegalArgumentException("Not valid Base64");

    int paddingNeeded = (4 - remainder) % 4;
    byte[] paddedBase64 = Arrays.copyOf(unpaddedBase64, unpaddedBase64.length + paddingNeeded);
    Arrays.fill(paddedBase64, unpaddedBase64.length, paddedBase64.length, (byte)'=');

    return paddedBase64;
}

[travisspencer]

Ah, good one @lachlan-roberts . That special case for a remainder of 1 took me a sec, but it's because a base64-endoded string will always be an even number. I see.

I think that an if statement like I have below is good to avoid the unnecessary array copy and I'd like to assign unpaddedBase64.length to a variable for readability. Will make those changes and update this PR.

[joakime]

Would like to see unit tests for the decoder portion, showing the various flavors of received data and to verify that decoder works as intended (and to prevent regression).

[lachlan-roberts]

I think the logic of padJWTSection looks good now.

I have written some tests as @joakime suggested and will put up a separate PR for them.

[travisspencer]

Ah great, @lachlan-roberts , that you wrote some tests as well. I had that on my todo list, but hadn't gotten to it yet. I'll check out your PR, so I can learn the details of how that's done.

With those, I think this PR is done.