-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Integer claim fails with GSON #616
Comments
This appears to be a GSON "bug" (at least in most peoples' opinion, mine included), but it's a perfectly fine 'feature' in the GSON maintainers' opinion. Honestly, I think they're being a bit difficult because the overwhelming majority of the Java community that uses GSON expects Jackson's saner default parsing heuristics. Here is the issue: google/gson#1084 That said, JJWT doesn't know how to perform type conversion for custom claims - just a few RFC standard claims - because custom type conversion is better suited for the parser, since it is the one responsible for type inference while parsing the JSON string. This means it's up to the JSON Parser (or you configuring the JSON Parser) to represent the object graph as you desire. This is why, for example, at least when using Jackson for custom claims typing, you can tell Jackson how to convert specific claims via https://github.com/jwtk/jjwt/tree/0.11.2#parsing-of-custom-claim-types Even in this case, JJWT doesn't know how to do the type conversion - the I'm not sure if you can do this with GSON because the GSON's But I wouldn't be comfortable adding that to JJWT's codebase since it is very GSON-specific and not even supported by their own API. Also, there are A LOT of people asking for similar fixes, and they almost all have different answers that "works for them" in their project. There doesn't seem to be a clean/generic solution for any project, making it even less likely that we should include an attempt in JJWT given the support burden it would put on us. It's just not a JWT concern, but a JSON parser concern. FWIW, this isn't the only time I've heard of people having problems with GSON. It's... fickle. 😞 I'd recommend Jackson if you can switch, or if not, try one of the workarounds on StackOverflow and then specify your custom/modified If you do find a clean/generic solution for Gson in a similar way of how we support Jackson and the |
This seems promising: https://stackoverflow.com/a/32235713/407170 |
Thanks, @lhazlewood As many people in the SO article say, and according to json.org and http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf (I assume these documents guide) there exists no specific number format in JSON. The documents state, that in JSON format a number can be expressed as integer, decimal or exponential (scientific notation). This given, I would think Let me say this again [and please correct me in case JWT specs say something different], a claim containing a number should always use double on both sides. How the long being converted to exponential fits in here, hmmm. The only thing I know from Google is they recommend specifying long numbers as string in JSON because a JS long does not have the full 32 bit as a Java long has. As JSON is derived from JS, again, it makes sense that an epoch long is parsed as exponential (floating point) by GSON. Not protecting Google here in any way, and also agreed with the point that adapters and settings should be available to interpret things in a specific programmable way. Only saying I can follow Google's rationing. |
The only purpose of a double or any floating point number is to represent fractional values. So if there is no fractional value - i.e. the value is a whole number - there is no point whatsoever in using a floating point number to represent integer whole values. The JSON spec indicates this in Section 8 (Numbers):
may is the key word. If there is no decimal point, there is no fractional part, and therefore there is no reason to represent the value in floating point representation. The JWT spec (RFC 7512, Section 2) states for claim values:
So both whole and fractional values are supported in JWT because they're supported in JSON. Based on this and our discussion, I'm even more convinced that the GSON team's stance on this is completely wrong, and here's two verifiable reasons:
Does that mean you shouldn't use GSON with JJWT? Not necessarily. If it were me, and I was forced to use GSON due to legacy code or a business/project policy (or similar), I would create the proper TypeAdapters as indicated in the SO links above to ensure that GSON behaved symmetrically. Only then would i feel comfortable using it to represent important data - especially JWT data which could have security implications.
It's not the only data type that can be used - and as indicated above it is imprecise. A simple check is all that's needed. For example GSON could have done something like this, and no-one would have these issues: JsonNumber number = parser.next();
String sval = number.stringValue();
if (sval.contains(".")) {
return toDecimal(sval); //returns a Float, Double or a BigDecimal depending on value size
} else {
return toInteger(sval); //returns an Integer, Long or BigInteger depending on value size
} If you really wanted to force a single data type that supports all three formats simultaneously, you could use BigDecimal, and that will not lose precision (unlike |
I thought I'd try some experiments. For posterity: private static boolean canBeDouble(Number number) {
return number instanceof BigDecimal &&
number.compareTo(BigDecimal.valueOf(Double.MIN_VALUE)) >= 0 &&
number.compareTo(BigDecimal.valueOf(Double.MAX_VALUE)) <= 0
}
private static boolean canBeFloat(Number number) {
return number instanceof BigDecimal &&
number.compareTo(BigDecimal.valueOf(Float.MIN_VALUE)) >= 0 &&
number.compareTo(BigDecimal.valueOf(Float.MAX_VALUE)) <= 0
}
private static boolean canBeLong(Number number) {
return number instanceof BigInteger &&
number.compareTo(BigInteger.valueOf(Long.MIN_VALUE)) >= 0 &&
number.compareTo(BigInteger.valueOf(Long.MAX_VALUE)) <= 0
}
private static boolean canBeInt(Number number) {
return number instanceof BigInteger &&
number.compareTo(BigInteger.valueOf(Integer.MIN_VALUE)) >= 0 &&
number.compareTo(BigInteger.valueOf(Integer.MAX_VALUE)) <= 0
}
private static String getIdealType(Number number) {
if (canBeInt(number)) {
return "Integer"
} else if (canBeLong(number)) {
return "Long"
} else if (canBeFloat(number)) {
return "Float"
} else if (canBeDouble(number)) {
return "Double"
} else if (number instanceof BigInteger) {
return "BigInteger";
} else {
return "BigDecimal";
}
}
private static void println(Number number) {
String sval;
if (number instanceof BigInteger) {
NumberFormat format = DecimalFormat.getIntegerInstance(Locale.US)
format.setGroupingUsed(false)
sval = format.format(number)
} else {
sval = ((BigDecimal)number).toPlainString()
}
println getIdealType(number) + ": " + sval
}
@Test
void testLimits() {
println BigInteger.valueOf(Long.MIN_VALUE).subtract(BigInteger.ONE) //less than Long can handle
println BigInteger.valueOf(Long.MIN_VALUE) //min Long can handle
println BigInteger.valueOf(Long.MIN_VALUE).add(BigInteger.ONE) //just before Long min
println BigInteger.valueOf(Integer.MIN_VALUE).subtract(BigInteger.ONE) //less than Integer can handle
println BigInteger.valueOf(Integer.MIN_VALUE) //min Integer
println BigInteger.valueOf(Integer.MIN_VALUE).add(BigInteger.ONE) //just before Integer min
println BigInteger.valueOf(Integer.MAX_VALUE).subtract(BigInteger.ONE) //just before Integer ax
println BigInteger.valueOf(Integer.MAX_VALUE) //max Integer
println BigInteger.valueOf(Integer.MAX_VALUE).add(BigInteger.ONE) //more than Integer can handle
println BigInteger.valueOf(Long.MAX_VALUE).subtract(BigInteger.ONE) //just before Long max
println BigInteger.valueOf(Long.MAX_VALUE) //max Long can handle
println BigInteger.valueOf(Long.MAX_VALUE).add(BigInteger.ONE) //more than Long can handle
println ''
println BigDecimal.valueOf(Double.MIN_VALUE).subtract(BigDecimal.valueOf(0.0000000001d))
println BigDecimal.valueOf(Double.MIN_VALUE)
println BigDecimal.valueOf(Float.MIN_VALUE).subtract(new BigDecimal(0.00000000000000000000000000000000000000000000000000000001d))
println BigDecimal.valueOf(Float.MIN_VALUE)
println BigDecimal.valueOf(Float.MAX_VALUE)
println BigDecimal.valueOf(Float.MAX_VALUE).add(new BigDecimal(0.00000000000000000000000000000000000000000000000000000001d))
println BigDecimal.valueOf(Double.MAX_VALUE)
println BigDecimal.valueOf(Double.MAX_VALUE).add(BigDecimal.valueOf(0.0000000001d))
} |
I agree with you that looking at the decimal point The problem with GSON I believe is that it doesn't have a definition of what's expected on the Java side, as we're using If I would write a JWT library with the things we have just found out, I would take the approach of defining an abstract As for my part, I've been using Jackson, but thanks for discussing. |
Yes, exactly. The GSON team's primary (exclusive?) focus is on marshaling and unmarshaling of strongly-typed Java classes, as opposed to Maps, so that leads to this issue.
Yes, that would indeed work in GSON, but poses a more invasive implementation requirement for all JJWT users: All users - not just GSON ones - would be forced to write implementation classes just to use JWTs, further coupling their codebase (and testing requirements) to JJWT's implementation details. That would have been a less-desirable outcome in my opinion than a general-purpose library that could create and parse JWTs with just library calls and without implementation requirements. The other driving force behind JJWT's approach is mostly the JWT RFC specifications. Those RFCs indicate that any header value or claims value can be present in a JWT, not just the 'standard' ones defined in the RFCs - i.e. header and claims can be completely arbitrary. This drove the current design since Maps are ideal to handle such variability. Another way of thinking about this: building a strongly-typed (non Map-based) And then what if that payload doesn't match an existing Java class perfectly because the codebase that generated the JWT is slightly different (different version) than the codebase that parses the JWT? Is a Also what about situations when 3rd party servers or services outside of your application are the ones that generate the JWT, but your application parses it? For example, a JWT could be created by an Identity Provider (e.g. such as Okta) or load balancers like Kong or Nginx or Amazon AWS ELB. When your app receives the JWT and parses it, but those servers/services change or add to the payload before your application can update its JWT implementation class - what happens? If JWTs were used purely in closed-loop systems or application architectures (where you control 100% of the codebases and versions all the time), maybe strongly-typed implementations could make sense, but many (most?) JWT usages don't really fall into that category. JJWT's Map-based approach avoids all of these potential problems. Was this the right approach? I think so, as JJWT is by a pretty far margin the most popular JWT library for the JVM. That said, it doesn't mean we can't do better, so there is a chance to improve the GSON discrepancy in the future. I'm assuming one good option here would be to allow a user to specify custom type adapters for any named field they desire, like we do with the Anyway, thanks for the engaging discussion! I hope this thread has been useful in understanding JJWT's behavior and design choices for you and any others that may find it in the future. I'm happy to discuss feedback and suggestions for improvement for this or anything else about JJWT 😄 |
I don't know if it's possible in Java to create a strongly typed class on the fly and pass it to GSON as Thanks for discussing as well, Les. :) |
This issue has been automatically marked as stale due to inactivity for 60 or more days. It will be closed in 7 days if no further activity occurs. |
Closed due to inactivity. |
This bug was closed in Gson library with this PR google/gson#1290 merged in version 2.9.0 of GSON library. Right now the right way to have numbers parsed correctly is to configure the gson() in this way. Gson gson = new GsonBuilder().setObjectToNumberStrategy(ToNumberPolicy.LONG_OR_DOUBLE).create(); I suggest to make a new release that relay on latest gson release (right now is 2.10.1). And to close the bug addid this line to the gson builder. Thanks. |
@patton73 thanks so much for the update! Glad they finally have a solution. Reopening here so we can address it in a future release. |
Ah, I just noticed you created a new issue for that. Closing this one in favor of #771 . Thanks! |
The following works with Jackson but fails with GSON because the claim value becomes non-integer during
build
.I have
I do
and then
The text was updated successfully, but these errors were encountered: