honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Improve error message reporting for invalid json - Fix #1793

This commit is contained in:
jamesagnew 2020-04-09 09:53:13 -04:00
parent 8cdc3a72ce
commit 2a43aa3b6c
8 changed files with 98 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseErrorHandler.java Normal file
View File

@ -0,0 +1,14 @@
package ca.uhn.fhir.parser;
class BaseErrorHandler {
String describeLocation(IParserErrorHandler.IParseLocation theLocation) {
if (theLocation == null) {
return "";
} else {
return theLocation.toString() + " ";
}
}
}

14
hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java
View File

@ -258,9 +258,9 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
break;
} else if (valueObj instanceof Long) {
if (theChildName != null) {
theEventWriter.write(theChildName, (long)valueObj);
theEventWriter.write(theChildName, (long) valueObj);
} else {
theEventWriter.write((long)valueObj);
theEventWriter.write((long) valueObj);
}
break;
}
@ -1387,10 +1387,6 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
}
}
private static void write(JsonLikeWriter theWriter, String theName, String theValue) throws IOException {
theWriter.write(theName, theValue);
}
private class HeldExtension implements Comparable<HeldExtension> {
private CompositeChildElement myChildElem;
@ -1562,7 +1558,7 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
if (childDef == null) {
throw new ConfigurationException("Unable to encode extension, unrecognized child element type: " + value.getClass().getCanonicalName());
}
encodeChildElementToStreamWriter(theResDef, theResource, theEventWriter, value, childDef, childName, false, myParent,false, theEncodeContext);
encodeChildElementToStreamWriter(theResDef, theResource, theEventWriter, value, childDef, childName, false, myParent, false, theEncodeContext);
managePrimitiveExtension(value, theResDef, theResource, theEventWriter, childDef, childName, theEncodeContext, theContainedResource);
theEncodeContext.popPath();
@ -1574,4 +1570,8 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
theEventWriter.endObject();
}
}
private static void write(JsonLikeWriter theWriter, String theName, String theValue) throws IOException {
theWriter.write(theName, theValue);
}
}

14
hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/LenientErrorHandler.java
View File

@ -38,7 +38,7 @@ import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType;
* @see IParser#setParserErrorHandler(IParserErrorHandler)
* @see FhirContext#setParserErrorHandler(IParserErrorHandler)
*/
public class LenientErrorHandler implements IParserErrorHandler {
public class LenientErrorHandler extends BaseErrorHandler implements IParserErrorHandler {
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(LenientErrorHandler.class);
private static final StrictErrorHandler STRICT_ERROR_HANDLER = new StrictErrorHandler();
@ -84,7 +84,7 @@ public class LenientErrorHandler implements IParserErrorHandler {
public void invalidValue(IParseLocation theLocation, String theValue, String theError) {
if (isBlank(theValue) || myErrorOnInvalidValue == false) {
if (myLogErrors) {
ourLog.warn("Invalid attribute value \"{}\": {}", theValue, theError);
ourLog.warn("{}Invalid attribute value \"{}\": {}", describeLocation(theLocation), theValue, theError);
}
} else {
STRICT_ERROR_HANDLER.invalidValue(theLocation, theValue, theError);
@ -133,35 +133,35 @@ public class LenientErrorHandler implements IParserErrorHandler {
@Override
public void unexpectedRepeatingElement(IParseLocation theLocation, String theElementName) {
if (myLogErrors) {
ourLog.warn("Multiple repetitions of non-repeatable element '{}' found while parsing", theElementName);
ourLog.warn("{}Multiple repetitions of non-repeatable element '{}' found while parsing", describeLocation(theLocation), theElementName);
}
}
@Override
public void unknownAttribute(IParseLocation theLocation, String theElementName) {
if (myLogErrors) {
ourLog.warn("Unknown attribute '{}' found while parsing", theElementName);
ourLog.warn("{}Unknown attribute '{}' found while parsing",describeLocation(theLocation), theElementName);
}
}
@Override
public void unknownElement(IParseLocation theLocation, String theElementName) {
if (myLogErrors) {
ourLog.warn("Unknown element '{}' found while parsing", theElementName);
ourLog.warn("{}Unknown element '{}' found while parsing", describeLocation(theLocation), theElementName);
}
}
@Override
public void unknownReference(IParseLocation theLocation, String theReference) {
if (myLogErrors) {
ourLog.warn("Resource has invalid reference: {}", theReference);
ourLog.warn("{}Resource has invalid reference: {}", describeLocation(theLocation), theReference);
}
}
@Override
public void extensionContainsValueAndNestedExtensions(IParseLocation theLocation) {
if (myLogErrors) {
ourLog.warn("Extension contains both a value and nested extensions: {}", theLocation);
ourLog.warn("Extension contains both a value and nested extensions: {}", describeLocation(theLocation));
}
}

9
hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParseLocation.java
View File

@ -54,6 +54,13 @@ class ParseLocation implements IParseLocation {
@Override
public String toString() {
return defaultString(myParentElementName);
return "[element=\"" + defaultString(myParentElementName) + "\"]";
}
/**
* Factory method
*/
static ParseLocation fromElementName(String theChildName) {
return new ParseLocation(theChildName);
}
}

31
hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java
View File

@ -191,7 +191,7 @@ class ParserState<T> {
return thePrimitiveTarget.newInstance(theDefinition.getInstanceConstructorArguments());
}
public IPrimitiveType<?> getPrimitiveInstance(BaseRuntimeChildDefinition theChild, RuntimePrimitiveDatatypeDefinition thePrimitiveTarget) {
public IPrimitiveType<?> getPrimitiveInstance(BaseRuntimeChildDefinition theChild, RuntimePrimitiveDatatypeDefinition thePrimitiveTarget, String theChildName) {
return thePrimitiveTarget.newInstance(theChild.getInstanceConstructorArguments());
}
@ -456,7 +456,7 @@ class ParserState<T> {
RuntimePrimitiveDatatypeDefinition primitiveTarget = (RuntimePrimitiveDatatypeDefinition) target;
IPrimitiveType<?> newChildInstance = newPrimitiveInstance(myDefinition, primitiveTarget);
myDefinition.getMutator().addValue(myParentInstance, newChildInstance);
PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance);
PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance, theLocalPart);
push(newState);
return;
}
@ -583,9 +583,9 @@ class ParserState<T> {
case PRIMITIVE_DATATYPE: {
RuntimePrimitiveDatatypeDefinition primitiveTarget = (RuntimePrimitiveDatatypeDefinition) target;
IPrimitiveType<?> newChildInstance;
newChildInstance = getPrimitiveInstance(child, primitiveTarget);
newChildInstance = getPrimitiveInstance(child, primitiveTarget, theChildName);
child.getMutator().addValue(myInstance, newChildInstance);
PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance);
PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance, theChildName);
push(newState);
return;
}
@ -752,7 +752,7 @@ class ParserState<T> {
RuntimePrimitiveDatatypeDefinition primitiveTarget = (RuntimePrimitiveDatatypeDefinition) target;
IPrimitiveType<?> newChildInstance = newInstance(primitiveTarget);
myExtension.setValue(newChildInstance);
PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance);
PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance, theLocalPart);
push(newState);
return;
}
@ -824,7 +824,7 @@ class ParserState<T> {
break;
case "lastUpdated":
InstantDt updated = new InstantDt();
push(new PrimitiveState(getPreResourceState(), updated));
push(new PrimitiveState(getPreResourceState(), updated, theLocalPart));
myMap.put(ResourceMetadataKeyEnum.UPDATED, updated);
break;
case "security":
@ -850,7 +850,7 @@ class ParserState<T> {
newProfiles = new ArrayList<>(1);
}
IdDt profile = new IdDt();
push(new PrimitiveState(getPreResourceState(), profile));
push(new PrimitiveState(getPreResourceState(), profile, theLocalPart));
newProfiles.add(profile);
myMap.put(ResourceMetadataKeyEnum.PROFILES, Collections.unmodifiableList(newProfiles));
break;
@ -891,7 +891,7 @@ class ParserState<T> {
private class MetaVersionElementState extends BaseState {
private ResourceMetadataMap myMap;
private final ResourceMetadataMap myMap;
MetaVersionElementState(ParserState<T>.PreResourceState thePreResourceState, ResourceMetadataMap theMap) {
super(thePreResourceState);
@ -1268,23 +1268,27 @@ class ParserState<T> {
}
private class PrimitiveState extends BaseState {
private final String myChildName;
private IPrimitiveType<?> myInstance;
PrimitiveState(PreResourceState thePreResourceState, IPrimitiveType<?> theInstance) {
PrimitiveState(PreResourceState thePreResourceState, IPrimitiveType<?> theInstance, String theChildName) {
super(thePreResourceState);
myInstance = theInstance;
myChildName = theChildName;
}
@Override
public void attributeValue(String theName, String theValue) throws DataFormatException {
if ("value".equals(theName)) {
if ("".equals(theValue)) {
myErrorHandler.invalidValue(null, theValue, "Attribute values must not be empty (\"\")");
ParseLocation location = ParseLocation.fromElementName(myChildName);
myErrorHandler.invalidValue(location, theValue, "Attribute value for element must not be empty (\"\")");
} else {
try {
myInstance.setValueAsString(theValue);
} catch (DataFormatException | IllegalArgumentException e) {
myErrorHandler.invalidValue(null, theValue, e.getMessage());
ParseLocation location = ParseLocation.fromElementName(myChildName);
myErrorHandler.invalidValue(location, theValue, e.getMessage());
}
}
} else if ("id".equals(theName)) {
@ -1295,7 +1299,8 @@ class ParserState<T> {
} else if (myInstance instanceof IBaseResource) {
new IdDt(theValue).applyTo((org.hl7.fhir.instance.model.api.IBaseResource) myInstance);
} else {
myErrorHandler.unknownAttribute(null, theName);
ParseLocation location = ParseLocation.fromElementName(myChildName);
myErrorHandler.unknownAttribute(location, theName);
}
} else {
super.attributeValue(theName, theValue);
@ -1343,7 +1348,7 @@ class ParserState<T> {
@Override
public void enteringNewElement(String theNamespace, String theChildName) throws DataFormatException {
if ("id".equals(theChildName)) {
push(new PrimitiveState(getPreResourceState(), myInstance.getId()));
push(new PrimitiveState(getPreResourceState(), myInstance.getId(), theChildName));
} else if ("meta".equals(theChildName)) {
push(new MetaElementState(getPreResourceState(), myInstance.getResourceMetadata()));
} else {

15
hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/StrictErrorHandler.java
View File

@ -3,6 +3,7 @@ package ca.uhn.fhir.parser;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.parser.json.JsonLikeValue.ScalarType;
import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType;
import ca.uhn.fhir.util.UrlUtil;
/*
* #%L
@ -31,7 +32,7 @@ import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType;
* @see IParser#setParserErrorHandler(IParserErrorHandler)
* @see FhirContext#setParserErrorHandler(IParserErrorHandler)
*/
public class StrictErrorHandler implements IParserErrorHandler {
public class StrictErrorHandler extends BaseErrorHandler implements IParserErrorHandler {
@Override
public void containedResourceWithNoId(IParseLocation theLocation) {
@ -46,7 +47,7 @@ public class StrictErrorHandler implements IParserErrorHandler {
@Override
public void invalidValue(IParseLocation theLocation, String theValue, String theError) {
throw new DataFormatException("Invalid attribute value \"" + theValue + "\": " + theError);
throw new DataFormatException(describeLocation(theLocation) + "Invalid attribute value \"" + UrlUtil.sanitizeUrlPart(theValue) + "\": " + theError);
}
@Override
@ -65,27 +66,27 @@ public class StrictErrorHandler implements IParserErrorHandler {
@Override
public void unexpectedRepeatingElement(IParseLocation theLocation, String theElementName) {
throw new DataFormatException("Multiple repetitions of non-repeatable element '" + theElementName + "' found during parse");
throw new DataFormatException(describeLocation(theLocation) + "Multiple repetitions of non-repeatable element '" + theElementName + "' found during parse");
}
@Override
public void unknownAttribute(IParseLocation theLocation, String theAttributeName) {
throw new DataFormatException("Unknown attribute '" + theAttributeName + "' found during parse");
throw new DataFormatException(describeLocation(theLocation) + "Unknown attribute '" + theAttributeName + "' found during parse");
}
@Override
public void unknownElement(IParseLocation theLocation, String theElementName) {
throw new DataFormatException("Unknown element '" + theElementName + "' found during parse");
throw new DataFormatException(describeLocation(theLocation) + "Unknown element '" + theElementName + "' found during parse");
}
@Override
public void unknownReference(IParseLocation theLocation, String theReference) {
throw new DataFormatException("Resource has invalid reference: " + theReference);
throw new DataFormatException(describeLocation(theLocation) + "Resource has invalid reference: " + theReference);
}
@Override
public void extensionContainsValueAndNestedExtensions(IParseLocation theLocation) {
throw new DataFormatException("Extension contains both a value and nested extensions: " + theLocation);
throw new DataFormatException(describeLocation(theLocation) + "Extension contains both a value and nested extensions: " + theLocation);
}
}

7
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1793-report-attr-name-in-json-parse-errors.yaml Normal file
View File

@ -0,0 +1,7 @@
---
type: add
issue: 1793
title: "When parsing JSON resources, if an element contains an invalid value, the Parser Error Handler
did not have access to the actual name of the element being parsed. This meant that errors lacked useful
detail in order to diagnose the issue. This has been corrected. Thanks to GitHub user
@jwalter for reporting!"

26
hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java
View File

@ -569,6 +569,32 @@ public class JsonParserR4Test extends BaseTest {
}
/**
* See #1793
*/
@Test
public void testParseEmptyAttribute() {
String input = "{\n" +
" \"resourceType\": \"Patient\",\n" +
" \"identifier\": [\n" +
" {\n" +
" \"system\": \"https://example.com\",\n" +
" \"value\": \"\"\n" +
" }\n" +
" ]\n" +
"}";
IParser jsonParser = ourCtx.newJsonParser();
jsonParser.setParserErrorHandler(new StrictErrorHandler());
try {
jsonParser.parseResource(Patient.class, input);
fail();
} catch (DataFormatException e) {
assertEquals("[element=\"value\"] Invalid attribute value \"\": Attribute value for element must not be empty (\"\")", e.getMessage());
}
}
@Test
public void testParseExtensionOnPrimitive() throws IOException {
String input = IOUtils.toString(JsonParserR4Test.class.getResourceAsStream("/extension-on-line.txt"), Constants.CHARSET_UTF8);