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

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

Closed
Wanjuuuuu opened this issue Jan 8, 2019 · 3 comments · Fixed by #1989

Comments

@Wanjuuuuu
Copy link

Hi,

while working on my project with Retrofit, I found an issue with newJsonWriter method in Gson class.

I used GsonConverterFactory which is a wrapper class of Gson in Retrofit.

Gson enables to escape HTML characters as a default.

However, Retrofit doesn't encode the request body as the same as Gson does.

Here is GsonConverterFactory code from Retrofit. I can see there is no any other options it gives to gson.

public final class GsonConverterFactory extends Converter.Factory {
  /**
   * Create an instance using a default {@link Gson} instance for conversion. Encoding to JSON and
   * decoding from JSON (when no charset is specified by a header) will use UTF-8.
   */
  public static GsonConverterFactory create() {
    return create(new Gson());
  }

  /**
   * Create an instance using {@code gson} for conversion. Encoding to JSON and
   * decoding from JSON (when no charset is specified by a header) will use UTF-8.
   */
  @SuppressWarnings("ConstantConditions") // Guarding public API nullability.
  public static GsonConverterFactory create(Gson gson) {
    if (gson == null) throw new NullPointerException("gson == null");
    return new GsonConverterFactory(gson);
  }

  private final Gson gson;

  private GsonConverterFactory(Gson gson) {
    this.gson = gson;
  }

  @Override
  public Converter<ResponseBody, ?> responseBodyConverter(Type type, Annotation[] annotations,
      Retrofit retrofit) {
    TypeAdapter<?> adapter = gson.getAdapter(TypeToken.get(type));
    return new GsonResponseBodyConverter<>(gson, adapter);
  }

  @Override
  public Converter<?, RequestBody> requestBodyConverter(Type type,
      Annotation[] parameterAnnotations, Annotation[] methodAnnotations, Retrofit retrofit) {
    TypeAdapter<?> adapter = gson.getAdapter(TypeToken.get(type));
    return new GsonRequestBodyConverter<>(gson, adapter);
  }
}
final class GsonRequestBodyConverter<T> implements Converter<T, RequestBody> {
  private static final MediaType MEDIA_TYPE = MediaType.get("application/json; charset=UTF-8");
  private static final Charset UTF_8 = Charset.forName("UTF-8");

  private final Gson gson;
  private final TypeAdapter<T> adapter;

  GsonRequestBodyConverter(Gson gson, TypeAdapter<T> adapter) {
    this.gson = gson;
    this.adapter = adapter;
  }

  @Override public RequestBody convert(T value) throws IOException {
    Buffer buffer = new Buffer();
    Writer writer = new OutputStreamWriter(buffer.outputStream(), UTF_8);
    JsonWriter jsonWriter = gson.newJsonWriter(writer);
    adapter.write(jsonWriter, value);
    jsonWriter.close();
    return RequestBody.create(MEDIA_TYPE, buffer.readByteString());
  }
}

As above, it uses gson.newJsonWriter(writer).

  /**
   * Returns a new JSON writer configured for the settings on this Gson instance.
   */
  public JsonWriter newJsonWriter(Writer writer) throws IOException {
    if (generateNonExecutableJson) {
      writer.write(JSON_NON_EXECUTABLE_PREFIX);
    }
    JsonWriter jsonWriter = new JsonWriter(writer);
    if (prettyPrinting) {
      jsonWriter.setIndent("  ");
    }
    jsonWriter.setSerializeNulls(serializeNulls);
    return jsonWriter;
  }

I look through newJsonWriter, but it does not apparently configure whole settings from Gson.

For example, it is HtmlSafe option in my case.

I guess HtmlSafe is garanteed as true in Gson, though, it can be false in JsonWriter when not using `setHtmlSafe(true)'.

Could you check this issue?

@tianxm
Copy link

tianxm commented Feb 26, 2019

This problem takes me a whole afternoon to find out where make it authorization error.

@sshock
Copy link

sshock commented May 9, 2020

This should really be fixed, but I have to admit that it worries me how many repercussions it might have for all projects currently using retrofit with GsonConverterFactory.create() to have an update at some point suddenly start escaping html characters when they are not used to having that happen because of this bug...

Of course, any sane json parser on the other end won't care either way, but you never know what brain-dead or broken parsers might exist on the other end.

But yeah, just a long way of saying, I really think this should be fixed, but I'm almost afraid to ask.

@snailflying
Copy link

Hi, can you fix it?

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 a pull request may close this issue.

4 participants