JWE arbitrary content compression

[mnylen]

The JWE content was compressed only when claims was used as payload, but when using arbitrary content string/bytes/input stream, the compression was omitted, while keeping the "zip" header field, leading to un-parseable JWE as the decompression failed on non-compressed content.

I couldn't find any evidence that this was intended behavior from the documentation or the JWE spec, so made a fix for this.

See #936

Note that people might have used a similar workaround as I presented in the discussion to manually compress the payload, so fixing this might result in some code breaking somewhere. Might be good to mention in the changelog.

[lhazlewood]

Excellent find!!! I can't believe this was missed, thank you so much 🙏

[lhazlewood]

We have to be careful here: if compression is not enabled, we don't want to write the plaintext Inputstream to an intermediate output stream (and then converted back into an input stream) which would be an unnecessary roundtrip/copy.

This logic should look more like that found in the sign method:

jjwt/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtBuilder.java

Lines 598 to 605 in c673b76

	if (payload.isClaims() || payload.isCompressed()) {
	ByteArrayOutputStream claimsOut = new ByteArrayOutputStream(8192);
	writeAndClose("JWS Unencoded Payload", payload, claimsOut);
	payloadStream = Streams.of(claimsOut.toByteArray());
	} else {
	// No claims and not compressed, so just get the direct InputStream:
	payloadStream = Assert.stateNotNull(payload.toInputStream(), "Payload InputStream cannot be null.");
	}

If possible, it'd be ideal if we can move this logic to a helper method that is shared by both sign and encrypt

[mnylen]

Makes sense.

I wonder though if we should also be a bit more clever with the initial byte array size for ByteArrayOutputStream. Seems a bit wasteful to do 8 KB byte array allocation if the content is only few hundred bytes. At least if the payload has byte array / string, we could use the initial payload size as the starting point, as even with compression, we shouldn't (at least usually) go above that size.

Edit: Well, maybe I'll try to make a different PR about the initial sizes later. Noticed that writeAndClose also creates a 4KB buffer, when it could also just use the initial payload size as the starting point.

[mnylen]

Done

[lhazlewood]

I'd like to retain the existing names in exception messages so we don't lose context, so I think the helper method needs to also accept a String identifying what is being converted (e.g. JWS Unencoded Payload, JWE Claims), e.g.

private InputStream toInputStream(Payload payload, String name) {
    ...
}

(also probably don't need the word Payload in the method name since it's redundant given it only accepts a Payload data type).

[lhazlewood]

Just a minor change remaining, and then I think we can merge. Thanks for your continued help!