Allow "optional-padding" for Base64Variant

[cowtowncoder]

(note: follow-up from FasterXML/jackson-databind#2183)

Currently Base64Variant instances either require use of specific padding character (and use it for output) or disable use of padding completely (neither written nor accepted).
But it seems that sometimes a more lenient variant would make sense: one that

1. Does write padding (with specific character) and
2. Accepts specific padding but does not require it (i.e. ok to omit)

It seems best to add a "mutant factory" method in Base64Variant instance (something like withLenientPadding(boolean) maybe?). Whether additional entries should be added Base64Variants is an open question.
We'll have to figure out what to do wrt existing "usePadding()" compared to new option; API may be cumbersome if we have 2 booleans but just 3 legal combinations.

Note that we may also change API for configuration for Jackson 3.x, later on.

[cowtowncoder]

This might be something for new contributors to tackle -- I can help as necessary.

[pavan-kalyan]

I think I can take a shot at this.
Based on preliminary understanding, basically we can either

• use withLenientPadding(true) to return the variant which does not require padding. (factory)
  or
• add a new variant to specifically handle the 2 points you have mentioned.

I'll take a deeper look into the wikipedia article to understand more.

[cowtowncoder]

@pavan-kalyan Yes, I think withLenienPadding() is needed at any rate (so one can modify all aspects separately, if necessary); but addition of a new variant(s) is optional. Latter because number of permutations can get big so it is not clear if new "standard" option makes sense.

I think starting with new method (and internal settings needed), implementation would be a good step: can then think about second aspect separately (can file a new issue if need be).

[pavan-kalyan]

I wanted to get a better idea on the invalid combination you mentioned because the API design would need to not let the user setup an invalid combination.

Considering usesPadding and lenientPadding as internal settings in Base64Variant, the following table is meant to explain how encoding and decoding would work.

usesPadding	lenientPadding	Encoding Behaviour	Decoding Behaviour
true	true	add padding	does not require padding
true	false	add padding	requires padding
false	true	do not add padding	should not have padding
false	false	do not add padding	does not require padding

which of the above would be the invalid combination, the 3rd one?

[cowtowncoder]

Hmmh. Spelled out like that, I wonder if there are more combinations -- leniency/non-leniency is not quite the same as required/not-allowed. Seems to me that instead of "lenient"/"strict" we have for reads

1. Require padding (in cases where content length not divisible by 3) -- not ok to omit
2. Do not allow padding (expect end-of-input or space characters) -- must omit
3. Allow both use and skipping of padding (true lenient)

which would either mean two bits for reading (accept / require), or use of Enum to indicate 3 choices.
I suppose enum way would seem cleaner.

If so, existing setting of usesPadding would translate into "use padding on writing", and in existing constructors should map to one of 3 choices for reading (if you write, you require on reading; if you do not write; do not accept?).

Besides this, we'd need new "mutant factory" method(s) (withPadding(writeOrNotBoolean, enumForReading)), for constructing new variants from 4 base ones.
Could additionally also have something like "withLenientPaddingRead()" to accept-but-not-require padding option? Easy enough, if so, to also add "withRequirePaddingRead()" / "withoutPaddingReading()" (or "withNoPaddingAccepted()".
Or, maybe, "withPaddingRequired()" / "withPaddingAllowed()" / "withPaddingForbidden()" trio.
And from that, instead of 2-argument method at all, "withWritePadding(boolean)".

Not sure if we should also add new variant(s) or not.

[pavan-kalyan]

Those are good ideas, I'll need to ponder on them and check for possible downsides for each.

Regarding adding new variants, what is the main benefit of adding a new variant? It would be easier to use right? so users wouldn't have to set internal settings, but it could lead a huge set of additional variants being added for all possible permutations of settings (assuming those permutations have a valid use case, which is a little hard to predict).

[cowtowncoder]

Correct: new variant would be mostly for convenience, and yes, number of permutations for all possibilities would be problematic.
Beyond convenience it is possible that some frameworks might only allow config of some settings via name (dynamic lookup for properties file) but not sure if that occurs for base64 setting (probably not a common config setting to expose that way).

[pavan-kalyan]

withPaddingRequired()/withPaddingAllowed()/ withPaddingForbidden() seem to be the most readable option.
How exactly is the user expected to interact with the Base64Variants though?
e.g

Base64Variant base64Variant = Base64Variants.MIME;
//This?
base64Variant = base64Variant.withPaddingForbidden();
//or This where it sets the setting required, (in which case should the name be changed to setPaddingForbidden?) 

base64Variant.withPaddingForbidden();

because it would determine the signature and usage of the methods.

[cowtowncoder]

I would think chaining would usually be used like

Base64Variant base64Variant = Base64Variants.MIME.withPaddingForbidden();

I specifically do not want to make Base64Variants mutable as they are immutable as that just opens up many new ways to get things to fail in interesting ways. :)

[pavan-kalyan]

The different reading behaviors require another constructor where the reading behaviour can be set, regardless of usesPadding/usesPaddingOnWriting.

could you elaborate on the default for existing constructors? I'm assuming existing constructors' signature should not be modified?

[cowtowncoder]

@pavan-kalyan Addition of new constructors would be needed yes, and expected. Existing constructors should be left for compatibility reasons, although may be deprecated if that makes sense (for eventual removal from 3.0).

I do not remember exact details of current set up but I think it is rigidly keeping settings so that if padding is used on writing, it must be found on reading too.