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

Gson doesn't deserialise Long numbers correctly #1084

Open
danieleguiducci opened this Issue May 23, 2017 · 16 comments

Comments

Projects
None yet
5 participants
@danieleguiducci

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

@lyubomyr-shaydariv

This comment has been minimized.

Show comment
Hide comment
@lyubomyr-shaydariv

lyubomyr-shaydariv May 24, 2017

Contributor

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());
Contributor

lyubomyr-shaydariv commented May 24, 2017

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());
@danieleguiducci

This comment has been minimized.

Show comment
Hide comment
@danieleguiducci

danieleguiducci May 25, 2017

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 :)

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 :)

@lyubomyr-shaydariv

This comment has been minimized.

Show comment
Hide comment
@lyubomyr-shaydariv

lyubomyr-shaydariv May 26, 2017

Contributor

@danieleguiducci Ah, I didn't notice the precision problems, sorry. Then you have just to use a type adapter like the above (exclude the Byte, Short, Integer, and Float parsers) that would implement the best number parsing strategy for you. :) IMHO, using such an adapter (as well as for Gson) would cause performance slowdown, so, I'd say you have to be free to choose what to pay for.

Contributor

lyubomyr-shaydariv commented May 26, 2017

@danieleguiducci Ah, I didn't notice the precision problems, sorry. Then you have just to use a type adapter like the above (exclude the Byte, Short, Integer, and Float parsers) that would implement the best number parsing strategy for you. :) IMHO, using such an adapter (as well as for Gson) would cause performance slowdown, so, I'd say you have to be free to choose what to pay for.

@danieleguiducci

This comment has been minimized.

Show comment
Hide comment
@danieleguiducci

danieleguiducci May 26, 2017

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

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

@mikhailmelnik

This comment has been minimized.

Show comment
Hide comment
@mikhailmelnik

mikhailmelnik Sep 8, 2017

Fighting with the same issue at the moment. The task is pretty simple - log responses with pretty print - but can't use GSON as it mangles unix timestamps. Other solutions have their own culprits (like JSONObject who escapes forward slashes without any way to stop it) but at least I don't have 2 converted to 2.0 and timestamps look normal not like 1.47668631713E12.

Fighting with the same issue at the moment. The task is pretty simple - log responses with pretty print - but can't use GSON as it mangles unix timestamps. Other solutions have their own culprits (like JSONObject who escapes forward slashes without any way to stop it) but at least I don't have 2 converted to 2.0 and timestamps look normal not like 1.47668631713E12.

@zenglian

This comment has been minimized.

Show comment
Hide comment
@zenglian

zenglian Mar 3, 2018

Contributor

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).

Contributor

zenglian commented Mar 3, 2018

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).

zenglian added a commit to zenglian/gson that referenced this issue Apr 2, 2018

Parse number to Integer, Long, BigInteger and Double respectively.
Floating number, scientific notation is parsed as double.
[-2147483648,2147483647] is parsed as Integer,
[-9223372036854775808,9223372036854775807] is parsed as Long,
Other integers are parsed as BigInteger.
It fixes related issues: #1264,#1092, #1084,#692,#525, #1084, etc
@boyan-velinov

This comment has been minimized.

Show comment
Hide comment
@boyan-velinov

boyan-velinov Apr 13, 2018

@zenglian Do you plan to make a pull request in original google repo?

@zenglian Do you plan to make a pull request in original google repo?

@lyubomyr-shaydariv

This comment has been minimized.

Show comment
Hide comment
@lyubomyr-shaydariv

lyubomyr-shaydariv Apr 13, 2018

Contributor

@zenglian Oh, you've quoted my comment not referring me directly.

Java does care the data type and size.

Yes, it does but you should not write heuristics at the adapter side trying to pick up the "best" type you can find for a particular number literal. Let me ask you: is 1

  • a Byte
  • a Short
  • an Integer
  • a Long
  • a Float
  • a Double
  • BigInteger
  • BigDecimal
  • AtomicInteger
  • AtomicLong
  • or any other Number from a third-party libraries like Google Guava, Apache Commons, etc?

Please how could you know the original type?. Let your mappings do and should declare the type themselves. For example, when you deserialize a list of users for List<Object>, why you don't write a heuristics type adapter to determine that this object may look like a user having a list of LinkedTreeMap instances? Therefore ArrayList<Object> is wrong for your case and for mine. Again, this is NOT a bug. Please read the discussion at #1267.

Contributor

lyubomyr-shaydariv commented Apr 13, 2018

@zenglian Oh, you've quoted my comment not referring me directly.

Java does care the data type and size.

Yes, it does but you should not write heuristics at the adapter side trying to pick up the "best" type you can find for a particular number literal. Let me ask you: is 1

  • a Byte
  • a Short
  • an Integer
  • a Long
  • a Float
  • a Double
  • BigInteger
  • BigDecimal
  • AtomicInteger
  • AtomicLong
  • or any other Number from a third-party libraries like Google Guava, Apache Commons, etc?

Please how could you know the original type?. Let your mappings do and should declare the type themselves. For example, when you deserialize a list of users for List<Object>, why you don't write a heuristics type adapter to determine that this object may look like a user having a list of LinkedTreeMap instances? Therefore ArrayList<Object> is wrong for your case and for mine. Again, this is NOT a bug. Please read the discussion at #1267.

@zenglian

This comment has been minimized.

Show comment
Hide comment
@zenglian

zenglian Apr 13, 2018

Contributor

It makes more sense to guess 1 as an integer number than a floating number. I believe most people will make such assumption. And in java int can be automatically cast to double.

List<Object> Map<x,Object> is quite common in java.

Contributor

zenglian commented Apr 13, 2018

It makes more sense to guess 1 as an integer number than a floating number. I believe most people will make such assumption. And in java int can be automatically cast to double.

List<Object> Map<x,Object> is quite common in java.

@boyan-velinov

This comment has been minimized.

Show comment
Hide comment
@boyan-velinov

boyan-velinov Apr 13, 2018

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}

boyan-velinov commented Apr 13, 2018

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}

@lyubomyr-shaydariv

This comment has been minimized.

Show comment
Hide comment
@lyubomyr-shaydariv

lyubomyr-shaydariv Apr 13, 2018

Contributor

And in java int can be automatically cast to double.

Only if these are primitives. You cannot cast java.lang.Double to java.lang.Integer and vice versa. That's why java.lang.Number is designed to be convertible to main numeric types in Java.

Seriously, why is 1 not a java.lang.Byte by this approach?

Contributor

lyubomyr-shaydariv commented Apr 13, 2018

And in java int can be automatically cast to double.

Only if these are primitives. You cannot cast java.lang.Double to java.lang.Integer and vice versa. That's why java.lang.Number is designed to be convertible to main numeric types in Java.

Seriously, why is 1 not a java.lang.Byte by this approach?

@zenglian

This comment has been minimized.

Show comment
Hide comment
@zenglian

zenglian Apr 13, 2018

Contributor

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.

Contributor

zenglian commented Apr 13, 2018

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.

@lyubomyr-shaydariv

This comment has been minimized.

Show comment
Hide comment
@lyubomyr-shaydariv

lyubomyr-shaydariv Apr 13, 2018

Contributor

@zenglian It would be nice if user requirements could always align with how JSON is designed. Please re-read the linked PR discussion explaining the cons of such a change and why it's not a bug. Thank you.

Contributor

lyubomyr-shaydariv commented Apr 13, 2018

@zenglian It would be nice if user requirements could always align with how JSON is designed. Please re-read the linked PR discussion explaining the cons of such a change and why it's not a bug. Thank you.

@zenglian

This comment has been minimized.

Show comment
Hide comment
@zenglian

zenglian Apr 14, 2018

Contributor

Since gson is for Java, I think it has to do some kind of adjustment. We do not need byte, but "long" should be fine for most integers (but it over flows for big integer -- it should be rare).

Maybe both sides can accept the solution that user can custom Object deserializer.

Contributor

zenglian commented Apr 14, 2018

Since gson is for Java, I think it has to do some kind of adjustment. We do not need byte, but "long" should be fine for most integers (but it over flows for big integer -- it should be rare).

Maybe both sides can accept the solution that user can custom Object deserializer.

@mikhailmelnik

This comment has been minimized.

Show comment
Hide comment
@mikhailmelnik

mikhailmelnik Apr 14, 2018

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.

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.

@zenglian

This comment has been minimized.

Show comment
Hide comment
@zenglian

zenglian Apr 15, 2018

Contributor

@lyubomyr-shaydariv Please note that in Java Double d = 1 is illegal while Double d = 1.0 is legal.

Below is from intellij idea.
qq 20180415111935

Again, JSON is not designed for Java while gson does.

Contributor

zenglian commented Apr 15, 2018

@lyubomyr-shaydariv Please note that in Java Double d = 1 is illegal while Double d = 1.0 is legal.

Below is from intellij idea.
qq 20180415111935

Again, JSON is not designed for Java while gson does.

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