INT-3904: Fix NPE in the JsonPropertyAccessor

JIRA: https://jira.spring.io/browse/INT-3904

When we use the same expression several times, the SpEL engine cache an `accessor` after the first use.
The next evaluation just bypass `canRead()` and in case of JSON that mean that we don't check that the income has the field or not.
For this case the `read()` must return `TypedValue.NULL` instead of just `null`.
This commit is contained in:
Artem Bilan
2015-12-08 15:37:38 -05:00
committed by Gary Russell
parent 65e2430079
commit 4606954cd2
2 changed files with 39 additions and 17 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2013-2014 the original author or authors.
* Copyright 2013-2015 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -18,6 +18,12 @@ package org.springframework.integration.json;
import java.io.IOException;
import org.springframework.expression.AccessException;
import org.springframework.expression.EvaluationContext;
import org.springframework.expression.PropertyAccessor;
import org.springframework.expression.TypedValue;
import org.springframework.util.Assert;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -25,12 +31,6 @@ import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ContainerNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import org.springframework.expression.AccessException;
import org.springframework.expression.EvaluationContext;
import org.springframework.expression.PropertyAccessor;
import org.springframework.expression.TypedValue;
import org.springframework.util.Assert;
/**
* A SpEL {@link PropertyAccessor} that knows how to read on Jackson JSON objects.
*
@@ -43,8 +43,8 @@ public class JsonPropertyAccessor implements PropertyAccessor {
/**
* The kind of types this can work with.
*/
private static final Class<?>[] SUPPORTED_CLASSES = new Class<?>[] {String.class, ToStringFriendlyJsonNode.class,
ObjectNode.class, ArrayNode.class};
private static final Class<?>[] SUPPORTED_CLASSES = new Class<?>[] { String.class, ToStringFriendlyJsonNode.class,
ObjectNode.class, ArrayNode.class };
// Note: ObjectMapper is thread-safe
private ObjectMapper objectMapper = new ObjectMapper();
@@ -66,8 +66,8 @@ public class JsonPropertyAccessor implements PropertyAccessor {
return (ContainerNode<?>) json;
}
else {
throw new AccessException("Can not act on json that is not a ContainerNode: "
+ json.getClass().getSimpleName());
throw new AccessException(
"Can not act on json that is not a ContainerNode: " + json.getClass().getSimpleName());
}
}
@@ -121,7 +121,7 @@ public class JsonPropertyAccessor implements PropertyAccessor {
return new TypedValue(wrap(json));
}
else {
return null;
return TypedValue.NULL;
}
}
}
@@ -155,6 +155,9 @@ public class JsonPropertyAccessor implements PropertyAccessor {
@Override
public String toString() {
if (node == null) {
return "null";
}
if (node.isValueNode()) {
// This is to avoid quotes around a TextNode for example
return node.asText();
@@ -166,14 +169,19 @@ public class JsonPropertyAccessor implements PropertyAccessor {
@Override
public boolean equals(Object o) {
return this == o
|| (!(o == null || getClass() != o.getClass())
&& this.node.equals(((ToStringFriendlyJsonNode) o).node));
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
ToStringFriendlyJsonNode that = (ToStringFriendlyJsonNode) o;
return (this.node == that.node) || (this.node != null && this.node.equals(that.node));
}
@Override
public int hashCode() {
return this.node.toString().hashCode();
return this.node != null ? this.node.toString().hashCode() : 0;
}
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2013-2014 the original author or authors.
* Copyright 2013-2015 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@
package org.springframework.integration.json;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
@@ -24,6 +25,7 @@ import org.hamcrest.Matchers;
import org.junit.Before;
import org.junit.Test;
import org.springframework.expression.Expression;
import org.springframework.expression.spel.SpelEvaluationException;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.expression.spel.support.StandardEvaluationContext;
@@ -119,6 +121,18 @@ public class JsonPropertyAccessorTests {
evaluate(json, "foo", Object.class);
}
@Test
public void testNoNullPointerWithCachedReadAccessor() throws Exception {
Expression expression = parser.parseExpression("foo");
Object json = mapper.readTree("{\"foo\": \"bar\"}");
Object value = expression.getValue(this.context, json);
assertThat(value, Matchers.instanceOf(JsonPropertyAccessor.ToStringFriendlyJsonNode.class));
assertEquals("bar", value.toString());
Object json2 = mapper.readTree("{}");
Object value2 = expression.getValue(this.context, json2);
assertNull(value2);
}
private <T> T evaluate(Object target, String expression, Class<T> expectedType) {
return parser.parseExpression(expression).getValue(context, target, expectedType);
}