gson: Gson doesn't deserialise Long numbers correctly

Gson shouldn’t cast a number to a Double if a number does not have decimal digits. It’s clearly wrong.

public class GsonVsJackson {


    public static void main(String[] args) throws Exception {
        testGson();
        System.out.println("========================");
        testJackson();
    }

    public static void testGson() {
        System.out.println("Testing Gson 2.8");
        Gson gson = new Gson();
        HashMap<String, Object> newTest = new HashMap<>();
        newTest.put("first", 6906764140092371368L);
        String jsonString = gson.toJson(newTest);
        System.out.println(jsonString); // output ok: {"first":6906764140092371368}
        Map<String, Object> mapFromJson = gson.fromJson(jsonString, Map.class);
        Number numberFromJson = (Number) mapFromJson.get("first");
        System.out.println(numberFromJson.getClass() + " = " + numberFromJson); // java.lang.Double val 6.9067641400923709E18
        long longVal = numberFromJson.longValue();
        System.out.println(longVal); // output rounded: 6906764140092370944
    }

    public static void testJackson() throws Exception {
        System.out.println("Testing Jackson");
        ObjectMapper jackson = new ObjectMapper();
        HashMap<String, Object> newTest = new HashMap<>();
        newTest.put("first", 6906764140092371368L);
        String jsonString = jackson.writeValueAsString(newTest);
        System.out.println(jsonString); // output ok: {"first":6906764140092371368}
        Map<String, Object> mapFromJson = jackson.readValue(jsonString, Map.class);
        Number numberFromJson = (Number) mapFromJson.get("first");
        System.out.println(numberFromJson.getClass() + " = " + numberFromJson); // java.math.BigInteger = 6906764140092371368
        long longVal = numberFromJson.longValue();
        System.out.println(longVal); // output OK: 6906764140092371368
    }

}

Kind Regards, Daniele

About this issue

  • Original URL
  • State: closed
  • Created 7 years ago
  • Reactions: 33
  • Comments: 38 (15 by maintainers)

Commits related to this issue

Most upvoted comments

It's not a bug, JSON does not distinguish between integers or floats, and does not care the size of numbers.

Gson is created for java not for javascript. Java does care the data type and size. JSON is not used for javascript only. Hotdog is not dog, there is no need to stick to its literal meaning. Number adapter won’t work for Object type, e.g., ArrayList<Object>.

This is an old issue existed for many years. It’s easy for to fix it. I hope it will be fixed in 2018. Other json libs do not have this bug (I think it is).

It’s not a bug, and gson does it fully legit: JSON does not distinguish between integers or floats, and does not care the size of numbers. Numerics in JSON are just numbers, and java.lang.Double is the best and largest primitive-counterpart candidate to hold a JSON number.

If you need a long value at a call-site, then just call its longValue() method. If, for any particular reason, you need a behavior you are talking about, then you have to implement a custom type adapter factory. Say, something like this (not sure if it’s implemented right, though):

final class BestNumberTypeAdapterFactory
		implements TypeAdapterFactory {

	private static final TypeAdapterFactory bestNumberTypeAdapterFactory = new BestNumberTypeAdapterFactory();

	private static final TypeToken<Number> numberTypeToken = new TypeToken<Number>() {
	};

	private BestNumberTypeAdapterFactory() {
	}

	static TypeAdapterFactory getBestNumberTypeAdapterFactory() {
		return bestNumberTypeAdapterFactory;
	}

	@Override
	public <T> TypeAdapter<T> create(final Gson gson, final TypeToken<T> typeToken) {
		if ( Number.class.isAssignableFrom(typeToken.getRawType()) ) {
			final TypeAdapter<Number> delegateNumberTypeAdapter = gson.getDelegateAdapter(this, numberTypeToken);
			final TypeAdapter<Number> numberTypeAdapter = new NumberTypeAdapter(delegateNumberTypeAdapter).nullSafe();
			@SuppressWarnings("unchecked")
			final TypeAdapter<T> typeAdapter = (TypeAdapter<T>) numberTypeAdapter;
			return typeAdapter;
		}
		return null;
	}

	private static final class NumberTypeAdapter
			extends TypeAdapter<Number> {

		private final TypeAdapter<Number> delegateNumberTypeAdapter;

		private NumberTypeAdapter(final TypeAdapter<Number> delegateNumberTypeAdapter) {
			this.delegateNumberTypeAdapter = delegateNumberTypeAdapter;
		}

		@Override
		public void write(final JsonWriter out, final Number value)
				throws IOException {
			delegateNumberTypeAdapter.write(out, value);
		}

		@Override
		public Number read(final JsonReader in)
				throws IOException {
			final String s = in.nextString();
			return parsers.stream()
					.map(parser -> parser.apply(s))
					.filter(Objects::nonNull)
					.findFirst()
					.get();
		}
	}

	private static <N extends Number> Function<String, N> parseOnNull(final Function<? super String, ? extends N> parser) {
		return s -> {
			try {
				return parser.apply(s);
			} catch ( final NumberFormatException ignored ) {
				return null;
			}
		};
	}

	private static final ImmutableList<Function<? super String, ? extends Number>> parsers = ImmutableList.<Function<? super String, ? extends Number>>builder()
			.add(parseOnNull(Byte::parseByte))
			.add(parseOnNull(Short::parseShort))
			.add(parseOnNull(Integer::parseInt))
			.add(parseOnNull(Long::parseLong))
			.add(parseOnNull(Float::parseFloat))
			.add(parseOnNull(Double::parseDouble))
			.build();

}

However, it can only work if your code can tell Gson to deserialize numbers. This won’t work:

@SuppressWarnings("unchecked")
final Map<String, Object> mapFromJson = jackson.readValue(jsonString, Map.class);

But this will:

private static final TypeToken<Map<String, Number>> stringToNumberMapTypeToken = new TypeToken<Map<String, Number>>() {
};
...
final Map<String, Object> mapFromJson = gson.fromJson(jsonString, stringToNumberMapTypeToken.getType());

I agree with the comment above and naturally we, as people, consider 1 as integer, not as rational or real.

The same should apply for the code as well. When there is a possibility to parse 1 it should be done to the most commonly used type, namely Integer. Of course, the value can be greater then Integer.MAX_VALUE, so in this case, it could be parsed to Long. And again if it is greater then Long.MAX_Value, the result will be Double.

At least fromJson() and toJson() applied consequently on one String, should result in the same string. Now, this is not the case.

Let’s give an example: Code:

public static void main(String[] args) {
        String jsonString = "{\"a\":1}";
        System.out.println("intput" + jsonString);
        Map<String, Object> jsonMap = new Gson().fromJson(jsonString, new TypeToken<Map<String, Object>>() {
        }.getType());
        System.out.println("output" + new Gson().toJson(jsonMap));
    }

Result:

intput{“a”:1} output{“a”:1.0}

If we are after user requirements my only one was to have json_in and json_out in json_in -> Object -> json_out conversion identical while current implementation completely breaks it.

Fixed by #1290.

Thank you @lyubomyr-shaydariv , I already solved the issue by replacing Gson with Jackson. I think that the correctness of an algorithm should has an higher priority over the performance.

I wrote this post hopping to help the Gson team to improve the quality of their library.

Have a good weekend!

Kind regards, Daniele

I don’t think this is bug, but the problem is that the NumberAdapter is not replaceable as the default adapters are hardcoded in UnmodifiableList. Ideally you should be able to remove current NumberAdapter and use your one. In my case, I needed this specifically:

               JsonToken.NUMBER       -> {
                    val value = reader.nextString()
                    return if (value.contains(".")) {
                        value.toDouble()
                    } else {
                        value.toLong()
                    }
                }

To achieve so, I use reflection to replace the adapter internally - it is super-ugly and bad, but it works:

class Gson {
    private val gson = com.google.gson.GsonBuilder().create()

    fun <T : Any> fromJson(payload: String, c: KClass<T>) = try {
        gson.fromJson(payload, c.java)
    } catch (e: Exception) {
        throw JsonSyntaxException("Invalid JSON: ${payload.truncate(128)}", e)
    }

    fun toJson(payload: Any) = gson.toJson(payload)


    class LongObjectTypeAdapter(private val gson: com.google.gson.Gson) : TypeAdapter<Any>() {
        override fun read(reader: JsonReader): Any? {
            val token = reader.peek()

            when (token) {
                JsonToken.BEGIN_ARRAY  -> {
                    val list = ArrayList<Any?>()
                    reader.beginArray()

                    while (reader.hasNext()) {
                        list.add(this.read(reader))
                    }

                    reader.endArray()
                    return list
                }
                JsonToken.BEGIN_OBJECT -> {
                    val map = LinkedTreeMap<String, Any?>()
                    reader.beginObject()

                    while (reader.hasNext()) {
                        map.put(reader.nextName(), this.read(reader))
                    }

                    reader.endObject()
                    return map
                }
                JsonToken.STRING       -> return reader.nextString()
                JsonToken.NUMBER       -> {
                    val value = reader.nextString()
                    return if (value.contains(".")) {
                        value.toDouble()
                    } else {
                        value.toLong()
                    }
                }

                JsonToken.BOOLEAN      -> return java.lang.Boolean.valueOf(reader.nextBoolean())
                JsonToken.NULL         -> {
                    reader.nextNull()
                    return null
                }
                else                   -> throw IllegalStateException()
            }
        }

        override fun write(out: JsonWriter, value: Any?) {
            if (value == null) {
                out.nullValue()
            } else {
                val typeAdapter = gson.getAdapter(value.javaClass)
                if (typeAdapter is LongObjectTypeAdapter) {
                    out.beginObject()
                    out.endObject()
                } else {
                    try {
                        typeAdapter.write(out, value)
                    } catch (e: Exception) {
                        // we don't care
                        out.nullValue()
                    }
                }
            }
        }
    }


    companion object {
        private val FACTORY = object : TypeAdapterFactory {
            override fun <T> create(gson: com.google.gson.Gson, type: TypeToken<T>): TypeAdapter<T>? {
                return if (type.rawType == Any::class.java) LongObjectTypeAdapter(gson) as TypeAdapter<T> else null
            }
        }

        init {
            try {
                val field = ObjectTypeAdapter::class.java.getDeclaredField("FACTORY")

                val modifiersField = Field::class.java.getDeclaredField("modifiers")
                modifiersField.isAccessible = true
                modifiersField.setInt(field, field.modifiers and Modifier.FINAL.inv())

                field.isAccessible = true
                field.set(null, FACTORY)
            } catch (e: Exception) {
                throw IllegalStateException(e)
            }
        }
    }
}

What happens if you specify types instead of using raw ArrayList instances? Ie. replace this:

		public ArrayList projects;
		public ArrayList versions; // optional

with this:

		public ArrayList<Project> projects;
		public ArrayList<Version> versions; // optional

This is not a debate contest, so do not try to find my logic hole. Instead, try to find out what is the user requirements. When the input is {“a”:1}, guess user most likely wants integer or float.

I’m not saying either that is not legit or it’s a bug, i’m saying that this can be done better without loosing precision, like Jackson does 😃