Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

in base64 encoding escape newlines only when between quotes #491

Closed
wants to merge 1 commit into from

Conversation

vboulaye
Copy link

This is an attempt to fix FasterXML/jackson-dataformats-text#90 which seems to have been introduced when switching the default Base64Variant to MIME instead of MIME_NO_LINEFEEDS (to fix FasterXML/jackson-dataformats-text#62)

When the encoded string is longer than 76 characters, the encoding method adds escaped '\n', instead of "real" newlines, in order to be able to include it in a quoted string.
But in the case of yaml, the encoding is done without enclosing quotes, and "real" newlines should be used for the decoding to work (as the "\n" is are not unescaped).

With this change, the newlines are only escaped when the encoding is done between quotes.

I tested the change by building jackson-databind and jackson-dataformat-texts (I do not know which other modules could be impacted by this change).

The additional test for encoding long arrays in jackson-dataformat-texts proposed in FasterXML/jackson-dataformats-text#90 also works with this change.

@cowtowncoder
Copy link
Member

I'll have to re-read description to fully understand the problem, but unfortunately I am not convinced the fix logic is safe: add-quotes does not have any semantic meaning that should make it safe (or not) of omitting escaping.
All it does is indicate, literally, whether double-quotes should be used to surround value: if not, it is quite possible caller might do enclosing, or it might be they are to be omitted. But it does not really indicate anything specific about expected handling.

Having said that I think that fix probably needs to be applied on YAML backend, if possible, even if it means code duplication (I am ok with cut'n paste as long as comments reflect where code came from).

@vboulaye
Copy link
Author

Ok, I was also afraid that it was a bit far fetched to do it here!

I'll try to do it directly in the YAMLGenerator in jackson-dataformats-text.

@vboulaye vboulaye closed this Oct 20, 2018
@vboulaye vboulaye deleted the long-binary-data-encoding branch October 20, 2018 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants