google / gson

A Java serialization/deserialization library to convert Java Objects into JSON and back
Apache License 2.0
23.4k stars 4.29k forks source link

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

Closed Wanjuuuuu closed 3 years ago

Wanjuuuuu commented 5 years ago

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 commented 5 years ago

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

sshock commented 4 years ago

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 commented 3 years ago

Hi, can you fix it?