[Java][okhttp] Fix gson deserialize format byte #7473
Conversation
…n) out of base64 string. Add disableHtmlEscaping() to gsonbuilder.
…t of byte array serialization and deserialization.
| @@ -37,6 +37,7 @@ import java.text.ParsePosition; | |||
| import java.time.LocalDate; | |||
| import java.time.OffsetDateTime; | |||
| import java.time.format.DateTimeFormatter; | |||
| import java.util.Base64; | |||
There was a problem hiding this comment.
Okhttp has a Base64 class that can be used instead of the Java8 one. This way android users can enjoy the fix.
| @@ -917,6 +917,10 @@ | |||
| "pending", | |||
| "sold" | |||
| ] | |||
| }, | |||
| "profilePhoto": { | |||
There was a problem hiding this comment.
This will impact all generators. So all examples shall be regenerated
There was a problem hiding this comment.
We want to keep petstore.json, petstore.yaml the same as the Petstore server (https://petstore.swagger.io) so please undo the change in these files.
I noticed that you already added the additional test case in petstore-with-fake-endpoints-models-for-testing.yaml (there's an similar test case here: https://github.com/benrobot/swagger-codegen/blob/53018c475ad6bbc097f29d47b11a2bc55b5142ea/modules/swagger-codegen/src/test/resources/2_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1202)
There was a problem hiding this comment.
Thank you. I'll undo the addition of Pet.profilePhoto and use the existing format_test.byte.
There was a problem hiding this comment.
Just to be clear. I am not regenerating all the clients because I've undone the changes instead.
| @@ -55,6 +56,9 @@ public class JSON { | |||
| private OffsetDateTimeTypeAdapter offsetDateTimeTypeAdapter = new OffsetDateTimeTypeAdapter(); | |||
| private LocalDateTypeAdapter localDateTypeAdapter = new LocalDateTypeAdapter(); | |||
| {{/jsr310}} | |||
| {{#java8}} | |||
| private LocalByteArrayAdapter localByteArrayAdapter = new LocalByteArrayAdapter(); | |||
There was a problem hiding this comment.
Why not just ByteArrayAdapter ?
There was a problem hiding this comment.
Agreed. I thought I was following the naming convention of the other type adapters but only the DateAdapter was prepended with Local.
| @@ -105,6 +109,10 @@ public class JSON { | |||
| .registerTypeAdapter(OffsetDateTime.class, offsetDateTimeTypeAdapter) | |||
| .registerTypeAdapter(LocalDate.class, localDateTypeAdapter) | |||
| {{/jsr310}} | |||
| {{#java8}} | |||
| .disableHtmlEscaping(){{! Added to prevent base64 = (equal sign) from being escaped }} | |||
There was a problem hiding this comment.
The gson default is to escape html (it is the contrary for Jackson) so this should be a choice left to the SDK user.
There was a problem hiding this comment.
Removed disableHtmlEscaping. Changed test to compensate.
| @Test | ||
| public void testApiClient() { | ||
| // the default api client is used |
There was a problem hiding this comment.
Something is wrong. The tests shouldn’t be deleted
There was a problem hiding this comment.
I will put these back.
There was a problem hiding this comment.
The deleted tests have been undeleted
…for base64 conversions instead of java 8 lib since the okhttp-gson client already includes okio. Remove setting to disableHtmlEscaping. Rename LocalByteArrayAdapter to ByteArrayAdapter.
…eady available under the format_test definition.
…erify exception since the server now returns a 500 for this request.
|
I see that CircleCI tests against a local instance of http://petstore.swagger.io. I will spin up my own instance so my tests run against that instead of the one published on the internet. |
…gerapi/petstore (heads up, this test fails if calling out to the internet published version of http://petstore.swagger.io)
|
@benrobot thanks for the PR, which has been merged into master. |
wing328
changed the title
|
@wing328 we're using the 3.0.0 branch but I'd like to have this fix. Is there a good way to do that? Can I propose a cherry-pick of this PR back into the Thanks! |
|
@HugoMario ping |
|
@wing328 Looks like this fix is still not in 3.0.11? |
The java client based off of okhttp-gson libraries failed to serialized and deserialize
type=string,format=byteThis PR aims to add byte[] serialization/deserialization functionality by adding a new Gson type adapter which employs the Base64 class available starting with Java 8
Unit testing showed that the default gson setup was also escaping the = (equal signs) commonly found at the end of a base64 string therefore, the disableHtmlEscaping option has been set via the gson builder
Fix #4824
cc: @bbdouglas, @JFCote, @sreeshas, @jfiala, @lukoyanov, @cbornet, @jeff9finger