Throw JWTDecodeException when date claim format is invalid #241
Conversation
| return null; | ||
| } | ||
| if (!node.canConvertToLong()) { | ||
| throw new JWTDecodeException(String.format("The claim '%s' contained an unexpected value.", claimName)); |
| Map<String, JsonNode> tree = new HashMap<>(); | ||
| TextNode node = new TextNode("123456789"); | ||
| tree.put("key", node); | ||
|
|
||
| Date date = deserializer.getDateFromSeconds(tree, "key"); | ||
| assertThat(date, is(nullValue())); | ||
| deserializer.getDateFromSeconds(tree, "key"); |
There was a problem hiding this comment.
So no assertions as now caught by the exception.expect? if getDateFromSeconds explodes?
There was a problem hiding this comment.
Exactly. Because the ExpectedException is "none" for every method but for those in which we override it, when we run tests they expect (or don't) to fail with X exception and Y message.
| @@ -65,9 +65,12 @@ public Payload deserialize(JsonParser p, DeserializationContext ctxt) throws IOE | |||
|
|
|||
| Date getDateFromSeconds(Map<String, JsonNode> tree, String claimName) { | |||
| JsonNode node = tree.get(claimName); | |||
| if (node == null || node.isNull() || !node.canConvertToLong()) { | |||
| if (node == null || node.isNull()) { | |||
There was a problem hiding this comment.
So non Java person here, what is the difference here in checking for null?
There was a problem hiding this comment.
node is the instance of the JsonNode we're checking. The check is asserting that the instance of this object is null OR that the parsed JSON value is null. This second check wouldn't be possible if the instance is null as that would throw a NPE when calling some method on it.
WDYT about the exception message?
Fixes #240