Fix: SDK not parsing headers with claims other than String - #127
Fix: SDK not parsing headers with claims other than String#127pmathew92 wants to merge 1 commit into
Conversation
| @@ -0,0 +1,109 @@ | |||
| # Design: Spec-complete JWT header parsing (issue #88) | |||
There was a problem hiding this comment.
@pmathew92 do we need to push this too?
There was a problem hiding this comment.
No... we don't want this
53ed992 to
cb167c5
Compare
pmathew92
changed the title
pmathew92
changed the title
| tree.put(entry.getKey(), new ClaimImpl(value)); | ||
| stringHeader.put(entry.getKey(), stringifyHeaderValue(value)); | ||
| } | ||
| header = Collections.unmodifiableMap(stringHeader); |
There was a problem hiding this comment.
Behavior change: getHeader() previously returned Gson's mutable map; it's now unmodifiable. Any caller that mutates the returned map will now hit UnsupportedOperationException. Since getHeader() is public API, call this out in the changelog/@deprecated note, or keep it mutable to stay source-compatible.
| if (value.isJsonPrimitive()) { | ||
| return value.getAsString(); | ||
| } | ||
| return value.toString(); |
There was a problem hiding this comment.
A JSON null header value (e.g. {"kid":null}) isn't a primitive, so this returns the literal string "null" instead of null. The old Map<String,String> deserialization stored an actual null. Add an isJsonNull() check
| signature = parts[2]; | ||
| } | ||
|
|
||
| private void parseHeader(String json) { |
There was a problem hiding this comment.
The catch (DecodeException e) { throw e; } then catch (Exception e) re-wrap pattern works but is a bit convoluted. Simpler: validate isJsonObject() outside the try, since fromJson(..., JsonElement.class) only throws on malformed JSON.
| private void parseHeader(String json) { | ||
| final JsonObject object; | ||
| try { | ||
| JsonElement element = getGson().fromJson(json, JsonElement.class); |
There was a problem hiding this comment.
getGson() registers the JWTDeserializer for JWTPayload, which is irrelevant for parsing the header into a raw JsonElement. A plain new Gson() (or reused static instance) is clearer and avoids the unnecessary type-adapter setup on every decode.
3 participants
Changes
This PR fixes the issue of DecodeException being thrown when the header of the token contains claims other than String types.
References
Fixes #88