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

Fix Gson.newJsonWriter ignoring lenient and HTML-safe setting #1989

Conversation

Marcono1234
Copy link
Collaborator

@Marcono1234 Marcono1234 commented Oct 11, 2021

Resolves #1452
Supersedes #1451

⚠️ This is a backward incompatible change because writers created by newJsonWriter will now by default produce HTML-safe output (because that is Gson's default setting). However, arguably the previous behavior was incorrect.

@google-cla google-cla bot added the cla: yes label Oct 11, 2021
@eamonnmcmanus
Copy link
Member

eamonnmcmanus commented Oct 15, 2021

This seems reasonable. Most likely people have been affected only by having to call setLenient etc on the returned JsonWriter even though they had already called it on the originating Gson object. I did try this change against the tests of all of Google's internal projects and saw no problems.

I think I would like to make a release before merging this change and #1992, since they are at least potentially breaking changes. And I'd like the OSGi fix from #1981 or an equivalent fix to be in that release. So it might be a little while before these are merged.

[@Marcono1234 could you e-mail me at my @google.com address?]

@Marcono1234
Copy link
Collaborator Author

Marcono1234 commented Oct 24, 2021

Most likely people have been affected only by having to call setLenient etc on the returned JsonWriter even though they had already called it on the originating Gson object.

The main problem is the HTML-safe setting: Gson has it enabled by default, but did not propagate it to the JsonWriter. So once this pull request is merged all usage of newJsonWriter will by default produce HTML-safe output (which was not the case before). This should not matter much because it is still valid JSON data, but maybe this causes issues in some situations. Retrofit seems to use that method so it might affect a large (?) number of users; however on the other hand some of them do want HTML-safe output, see also the description of #1452.

So it would be good indeed to have that in a separate release.

@eamonnmcmanus
Copy link
Member

eamonnmcmanus commented Nov 1, 2021

The 2.8.9 release has now happened so we can merge this as part of the future 2.9.0.

@eamonnmcmanus eamonnmcmanus merged commit deaa3a6 into google:master Nov 1, 2021
3 checks passed
@Marcono1234 Marcono1234 deleted the marcono1234/Gson-newJsonWriter-improvements branch Nov 1, 2021
tibor-universe pushed a commit to getuniverse/gson that referenced this pull request Nov 21, 2021
…le#1989)

* Improve Gson newJsonWriter and newJsonReader documentation

* Consider lenient and HTML-safe setting for Gson.newJsonWriter

* Remove empty line between imports
@Marcono1234 Marcono1234 mentioned this pull request Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

newJsonWriter not return a new JSON writer configured for the settings on this Gson instance!
2 participants