Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Java] Configuration option to disable HTML escaping when using Gson #7966

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

Merged
HugoMario merged 3 commits into swagger-api:master from stianlik:gson-disableHtmlEscaping
Jun 11, 2018
Merged

[Java] Configuration option to disable HTML escaping when using Gson #7966

HugoMario merged 3 commits into swagger-api:master from stianlik:gson-disableHtmlEscaping
Jun 11, 2018

Conversation

@stianlik
Copy link
Contributor

@stianlik stianlik commented Apr 4, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language. @bbdouglas (2017/07) @JFCote (2017/08) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09) @jeff9finger (2018/01)

Description of the PR

The default implementation of Gson will escape certain characters. This includes the = character, which is used in base64 encoding and cause problems when deserializing the value to a base64 encoded string in a service.

Adding an option for disabling this feature makes it easier to generate client code with sane defaults.

eivinhb and ftlno reacted with thumbs up emoji
stianlik added 2 commits April 4, 2018 13:18
The default implementation of Gson will escape certain characters by default. This
includes the `=` character, which is used in base64 encoding and cause problems when
deserializing the value to a base64 encoded string in a service.
Adding an option for disabling this feature makes it easier to generate client code
with sane defaults.
@stianlik stianlik changed the title (削除) Configuration option to disable HTML escaping when using Gson (削除ここまで) (追記) [Java] Configuration option to disable HTML escaping when using Gson (追記ここまで) Apr 4, 2018
Copy link

I'd like to see more documentation describing the need and what is done. It takes some looking around to figure it out.

Copy link
Contributor Author

stianlik commented Apr 10, 2018
edited
Loading

This is a small change, but generating the Petstore example created a large diff. To see what this PR contains, it's easiest to view the relevant commit directly.

What do this PR include

A new option -DdisableHtmlEscaping=true, that can be used when generating Java-code using Gson in order to disable escaping of string values. Mainly to avoid escaping the = character when working with base64-encoded data. The option is documented as a configuration property (swagger-gen config-help -l java).

Why is this needed

When base64 support was implemented, the author initially disabled the Gson default for strings, which is to escape characters that could cause problems in an XML document. He did this because the escape feature caused problems with the = character, and resulted in invalid base64 code. I considered marking this as a bug, and implementing the fix without an option as per the original PR, but I chose to regard it as a feature request instead since it seemed like the preferred option was to keep the defaults for Gson. This preserves backwards compability and allow users to override the default setting instead of changing the existing behavior.

According to RFC 4648, base64 use a limited character subset, in addition to the = character.

A 65-character subset of US-ASCII is used, enabling 6 bits to be represented per printable character. (The extra 65th character, "=", is used to signify a special processing function.)

This means that ABC== is a valid base64 encoded string. However, the default behavior for Gson is to accommodate XML output by escaping certain characters, so we end up with ABC\u003d\u003d, which is not a valid base64 string, unless the receiver first unescape the = character.

I encountered this problem when I integrated with a service that accepts base64, processed through Spring and Jackson. When we send base64 data to the service, it works just fine. However, when the = signs are escaped, we (correctly) get a mapping error. I temporarily resolved the issue by modifying the generated client code and adding builder.disableHtmlEscaping() to disable this behavior. This works, for now, but we have to modify the generated code every time we make a change to the service. Instead, it would be very helpful to have an option to disable this behavior when we generate the code, which is what this PR provides.

Copy link
Contributor Author

@jeff9finger Do you need anything more to review the pull request?

Copy link

Great explanation and documentation!
LGTM 👍

Copy link

@jeff9finger jeff9finger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor Author

@HugoMario Any possibility for merging this?

Copy link
Contributor

yes, sorry for delay, thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jeff9finger jeff9finger jeff9finger approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /