Make the parser configurable so that when parsing an invalid empty value (e.g. {"status":""} parser will either throw a meaningful exception or log a warning depending on the configured error handler.

This commit is contained in:
James Agnew 2016-11-21 14:48:44 +01:00
parent 285b8aa1dd
commit 5846ce4518
11 changed files with 166 additions and 90 deletions

View File

@ -62,4 +62,9 @@ public class ErrorHandlerAdapter implements IParserErrorHandler {
// NOP
}
@Override
public void invalidValue(IParseLocation theLocation, String theValue, String theError) {
// NOP
}
}

View File

@ -50,6 +50,17 @@ public interface IParserErrorHandler {
*/
void incorrectJsonType(IParseLocation theLocation, String theElementName, ValueType theExpected, ValueType theFound);
/**
* The parser detected an atttribute value that was invalid (such as: empty "" values are not permitted)
*
* @param theLocation
* The location in the document. Note that this may be <code>null</code> as the ParseLocation feature is experimental. Use with caution, as the API may change.
* @param theValue The actual value
* @param theError A description of why the value was invalid
* @since 2.2
*/
void invalidValue(IParseLocation theLocation, String theValue, String theError);
/**
* Resource was missing a required element
*

View File

@ -1087,7 +1087,7 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
// return object;
// }
private void parseAlternates(JsonLikeValue theAlternateVal, ParserState<?> theState, String theElementName) {
private void parseAlternates(JsonLikeValue theAlternateVal, ParserState<?> theState, String theElementName, String theAlternateName) {
if (theAlternateVal == null || theAlternateVal.isNull()) {
return;
}
@ -1100,11 +1100,17 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
if (array.size() == 0) {
return;
}
parseAlternates(array.get(0), theState, theElementName);
parseAlternates(array.get(0), theState, theElementName, theAlternateName);
return;
}
JsonLikeObject alternate = theAlternateVal.getAsObject();
JsonLikeValue alternateVal = theAlternateVal;
if (alternateVal != null && alternateVal.isObject() == false) {
getErrorHandler().incorrectJsonType(null, theAlternateName, ValueType.OBJECT, alternateVal.getJsonType());
return;
}
JsonLikeObject alternate = alternateVal.getAsObject();
for (String nextKey : alternate.keySet()) {
JsonLikeValue nextVal = alternate.get(nextKey);
if ("extension".equals(nextKey)) {
@ -1280,11 +1286,6 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
handledUnderscoreNames++;
}
if (alternateVal != null && alternateVal.isObject() == false) {
getErrorHandler().incorrectJsonType(null, alternateName, ValueType.OBJECT, alternateVal.getJsonType());
alternateVal = null;
}
parseChildren(theState, nextName, nextVal, alternateVal, alternateName);
}
@ -1313,7 +1314,7 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
String nextName = alternateName.substring(1);
if (theObject.get(nextName) == null) {
theState.enteringNewElement(null, nextName);
parseAlternates(nextValue, theState, alternateName);
parseAlternates(nextValue, theState, alternateName, alternateName);
theState.endingElement();
}
} else {
@ -1329,7 +1330,14 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
private void parseChildren(ParserState<?> theState, String theName, JsonLikeValue theJsonVal, JsonLikeValue theAlternateVal, String theAlternateName) {
if (theJsonVal.isArray()) {
JsonLikeArray nextArray = theJsonVal.getAsArray();
JsonLikeArray nextAlternateArray = JsonLikeValue.asArray(theAlternateVal); // could be null
JsonLikeValue alternateVal = theAlternateVal;
if (alternateVal != null && alternateVal.isArray() == false) {
getErrorHandler().incorrectJsonType(null, theAlternateName, ValueType.ARRAY, alternateVal.getJsonType());
alternateVal = null;
}
JsonLikeArray nextAlternateArray = JsonLikeValue.asArray(alternateVal); // could be null
for (int i = 0; i < nextArray.size(); i++) {
JsonLikeValue nextObject = nextArray.get(i);
JsonLikeValue nextAlternate = null;
@ -1340,7 +1348,7 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
}
} else if (theJsonVal.isObject()) {
theState.enteringNewElement(null, theName);
parseAlternates(theAlternateVal, theState, theAlternateName);
parseAlternates(theAlternateVal, theState, theAlternateName, theAlternateName);
JsonLikeObject nextObject = theJsonVal.getAsObject();
boolean preResource = false;
if (theState.isPreResource()) {
@ -1358,13 +1366,13 @@ public class JsonParser extends BaseParser implements IJsonLikeParser {
theState.endingElement();
} else if (theJsonVal.isNull()) {
theState.enteringNewElement(null, theName);
parseAlternates(theAlternateVal, theState, theAlternateName);
parseAlternates(theAlternateVal, theState, theAlternateName, theAlternateName);
theState.endingElement();
} else {
// must be a SCALAR
theState.enteringNewElement(null, theName);
theState.attributeValue("value", theJsonVal.getAsString());
parseAlternates(theAlternateVal, theState, theAlternateName);
parseAlternates(theAlternateVal, theState, theAlternateName, theAlternateName);
theState.endingElement();
}
}

View File

@ -65,6 +65,13 @@ public class LenientErrorHandler implements IParserErrorHandler {
}
}
@Override
public void invalidValue(IParseLocation theLocation, String theValue, String theError) {
if (myLogErrors) {
ourLog.warn("Invalid attribute value \"{}\": {}", theValue, theError);
}
}
@Override
public void missingRequiredElement(IParseLocation theLocation, String theElementName) {
if (myLogErrors) {

View File

@ -20,17 +20,10 @@ package ca.uhn.fhir.parser;
* #L%
*/
import static org.apache.commons.lang3.StringUtils.defaultIfBlank;
import static org.apache.commons.lang3.StringUtils.defaultString;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
import static org.apache.commons.lang3.StringUtils.isNotEmpty;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.*;
import javax.xml.stream.events.StartElement;
import javax.xml.stream.events.XMLEvent;
@ -38,50 +31,11 @@ import javax.xml.stream.events.XMLEvent;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.Validate;
import org.apache.commons.lang3.tuple.Pair;
import org.hl7.fhir.instance.model.api.IAnyResource;
import org.hl7.fhir.instance.model.api.IBase;
import org.hl7.fhir.instance.model.api.IBaseBackboneElement;
import org.hl7.fhir.instance.model.api.IBaseBinary;
import org.hl7.fhir.instance.model.api.IBaseBundle;
import org.hl7.fhir.instance.model.api.IBaseElement;
import org.hl7.fhir.instance.model.api.IBaseExtension;
import org.hl7.fhir.instance.model.api.IBaseHasExtensions;
import org.hl7.fhir.instance.model.api.IBaseHasModifierExtensions;
import org.hl7.fhir.instance.model.api.IBaseMetaType;
import org.hl7.fhir.instance.model.api.IBaseReference;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IBaseXhtml;
import org.hl7.fhir.instance.model.api.ICompositeType;
import org.hl7.fhir.instance.model.api.IDomainResource;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.instance.model.api.IPrimitiveType;
import org.hl7.fhir.instance.model.api.*;
import ca.uhn.fhir.context.BaseRuntimeChildDefinition;
import ca.uhn.fhir.context.*;
import ca.uhn.fhir.context.BaseRuntimeChildDefinition.IMutator;
import ca.uhn.fhir.context.BaseRuntimeElementCompositeDefinition;
import ca.uhn.fhir.context.BaseRuntimeElementDefinition;
import ca.uhn.fhir.context.ConfigurationException;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.context.FhirVersionEnum;
import ca.uhn.fhir.context.RuntimeChildDeclaredExtensionDefinition;
import ca.uhn.fhir.context.RuntimePrimitiveDatatypeDefinition;
import ca.uhn.fhir.context.RuntimePrimitiveDatatypeNarrativeDefinition;
import ca.uhn.fhir.context.RuntimePrimitiveDatatypeXhtmlHl7OrgDefinition;
import ca.uhn.fhir.context.RuntimeResourceBlockDefinition;
import ca.uhn.fhir.context.RuntimeResourceDefinition;
import ca.uhn.fhir.model.api.BaseBundle;
import ca.uhn.fhir.model.api.Bundle;
import ca.uhn.fhir.model.api.BundleEntry;
import ca.uhn.fhir.model.api.ExtensionDt;
import ca.uhn.fhir.model.api.IElement;
import ca.uhn.fhir.model.api.IFhirVersion;
import ca.uhn.fhir.model.api.IIdentifiableElement;
import ca.uhn.fhir.model.api.IPrimitiveDatatype;
import ca.uhn.fhir.model.api.IResource;
import ca.uhn.fhir.model.api.ISupportsUndeclaredExtensions;
import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum;
import ca.uhn.fhir.model.api.Tag;
import ca.uhn.fhir.model.api.TagList;
import ca.uhn.fhir.model.api.*;
import ca.uhn.fhir.model.base.composite.BaseResourceReferenceDt;
import ca.uhn.fhir.model.base.resource.ResourceMetadataMap;
import ca.uhn.fhir.model.primitive.IdDt;
@ -2352,7 +2306,11 @@ class ParserState<T> {
@Override
public void attributeValue(String theName, String theValue) throws DataFormatException {
if ("value".equals(theName)) {
myInstance.setValueAsString(theValue);
if ("".equals(theValue)) {
myErrorHandler.invalidValue(null, theValue, "Attribute values must not be empty (\"\")");
} else {
myInstance.setValueAsString(theValue);
}
} else if ("id".equals(theName)) {
if (myInstance instanceof IIdentifiableElement) {
((IIdentifiableElement) myInstance).setElementSpecificId(theValue);

View File

@ -13,7 +13,7 @@ import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType;
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
@ -24,7 +24,7 @@ import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType;
*/
/**
* Parser error handler which throws a {@link DataFormatException} any time an
* Parser error handler which throws a {@link DataFormatException} any time an
* issue is found while parsing.
*
* @see IParser#setParserErrorHandler(IParserErrorHandler)
@ -32,29 +32,19 @@ import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType;
*/
public class StrictErrorHandler implements IParserErrorHandler {
@Override
public void unknownElement(IParseLocation theLocation, String theElementName) {
throw new DataFormatException("Unknown element '" + theElementName + "' found during parse");
}
@Override
public void unknownAttribute(IParseLocation theLocation, String theAttributeName) {
throw new DataFormatException("Unknown attribute '" + theAttributeName + "' found during parse");
}
@Override
public void unexpectedRepeatingElement(IParseLocation theLocation, String theElementName) {
throw new DataFormatException("Multiple repetitions of non-repeatable element '" + theElementName + "' found during parse");
}
@Override
public void containedResourceWithNoId(IParseLocation theLocation) {
throw new DataFormatException("Resource has contained child resource with no ID");
}
@Override
public void unknownReference(IParseLocation theLocation, String theReference) {
throw new DataFormatException("Resource has invalid reference: " + theReference);
public void incorrectJsonType(IParseLocation theLocation, String theElementName, ValueType theExpected, ValueType theFound) {
throw new DataFormatException("Found incorrect type for element " + theElementName + " - Expected " + theExpected.name() + " and found " + theFound.name());
}
@Override
public void invalidValue(IParseLocation theLocation, String theValue, String theError) {
throw new DataFormatException("Invalid attribute value \"" + theValue + "\": " + theError);
}
@Override
@ -72,9 +62,23 @@ public class StrictErrorHandler implements IParserErrorHandler {
}
@Override
public void incorrectJsonType(IParseLocation theLocation, String theElementName, ValueType theExpected, ValueType theFound) {
throw new DataFormatException("Found incorrect type for element " + theElementName + " - Expected " + theExpected.name() + " and found " + theFound.name());
public void unexpectedRepeatingElement(IParseLocation theLocation, String theElementName) {
throw new DataFormatException("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");
}
@Override
public void unknownElement(IParseLocation theLocation, String theElementName) {
throw new DataFormatException("Unknown element '" + theElementName + "' found during parse");
}
@Override
public void unknownReference(IParseLocation theLocation, String theReference) {
throw new DataFormatException("Resource has invalid reference: " + theReference);
}
}

View File

@ -17,14 +17,12 @@ import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.servlet.FilterHolder;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.hl7.fhir.dstu3.hapi.validation.DefaultProfileValidationSupport;
import org.hl7.fhir.dstu3.model.Bundle;
import org.hl7.fhir.dstu3.model.Patient;
import org.hl7.fhir.dstu3.model.Bundle.BundleEntryComponent;
import org.hl7.fhir.dstu3.model.Patient;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
import org.springframework.jdbc.support.incrementer.MySQLMaxValueIncrementer;
import org.springframework.web.context.ContextLoader;
import org.springframework.web.context.WebApplicationContext;
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
@ -41,8 +39,6 @@ import ca.uhn.fhir.narrative.DefaultThymeleafNarrativeGenerator;
import ca.uhn.fhir.rest.client.IGenericClient;
import ca.uhn.fhir.rest.client.ServerValidationModeEnum;
import ca.uhn.fhir.rest.client.interceptor.LoggingInterceptor;
import ca.uhn.fhir.rest.server.CORSFilter_;
import ca.uhn.fhir.rest.server.FifoMemoryPagingProvider;
import ca.uhn.fhir.rest.server.RestfulServer;
import ca.uhn.fhir.util.TestUtil;

View File

@ -144,6 +144,42 @@ public class JsonParserDstu2Test {
assertEquals(0, b.getEntry().size());
}
/**
* #480
*/
@Test
public void testEncodeEmptyValue() {
QuestionnaireResponse qr = new QuestionnaireResponse();
qr.setId("123");
qr.getAuthoredElement().setValueAsString("");
qr.getGroup().setLinkId(new StringDt());
qr.getGroup().addQuestion().setLinkId(new StringDt(""));
qr.getGroup().addQuestion().setLinkId(new StringDt("LINKID"));
String encoded = ourCtx.newJsonParser().encodeResourceToString(qr);
ourLog.info(encoded);
assertThat(encoded, stringContainsInOrder("123"));
assertThat(encoded, not(stringContainsInOrder("\"\"")));
assertThat(encoded, not(stringContainsInOrder("null")));
}
/**
* #480
*/
@Test
public void testParseEmptyValue() {
String input = "{\"resourceType\":\"QuestionnaireResponse\",\"id\":\"123\",\"authored\":\"\",\"group\":{\"linkId\":\"\"}}";
QuestionnaireResponse qr = ourCtx.newJsonParser().parseResource(QuestionnaireResponse.class, input);
assertEquals("QuestionnaireResponse/123", qr.getIdElement().getValue());
assertEquals(null, qr.getAuthored());
assertEquals(null, qr.getAuthoredElement().getValue());
assertEquals(null, qr.getAuthoredElement().getValueAsString());
assertEquals(null, qr.getGroup().getLinkId());
assertEquals(null, qr.getGroup().getLinkIdElement().getValue());
}
@Test
public void testEncodeAndParseExtensions() throws Exception {

View File

@ -33,6 +33,7 @@ public class ErrorHandlerTest {
new ErrorHandlerAdapter().unknownReference(null, null);
new ErrorHandlerAdapter().missingRequiredElement(null, null);
new ErrorHandlerAdapter().incorrectJsonType(null, null, null, null);
new ErrorHandlerAdapter().invalidValue(null, null, null);
}
@Test
@ -43,6 +44,7 @@ public class ErrorHandlerTest {
new LenientErrorHandler().containedResourceWithNoId(null);
new LenientErrorHandler().unknownReference(null, null);
new LenientErrorHandler().incorrectJsonType(null, null, null, null);
new LenientErrorHandler().invalidValue(null, null, null);
}
@Test(expected = DataFormatException.class)
@ -75,4 +77,9 @@ public class ErrorHandlerTest {
new StrictErrorHandler().incorrectJsonType(null, null, null, null);
}
@Test(expected = DataFormatException.class)
public void testStrictMethods7() {
new StrictErrorHandler().invalidValue(null, null, null);
}
}

View File

@ -66,6 +66,42 @@ public class JsonParserDstu3Test {
ourCtx.setNarrativeGenerator(null);
}
/**
* #480
*/
@Test
public void testEncodeEmptyValue() {
QuestionnaireResponse qr = new QuestionnaireResponse();
qr.setId("123");
qr.getAuthoredElement().setValueAsString("");
qr.getItemFirstRep().setLinkIdElement(new StringType());
qr.getItemFirstRep().addItem().setLinkIdElement(new StringType(""));
qr.getItemFirstRep().addItem().setLinkIdElement(new StringType("LINKID"));
String encoded = ourCtx.newJsonParser().encodeResourceToString(qr);
ourLog.info(encoded);
assertThat(encoded, stringContainsInOrder("123"));
assertThat(encoded, not(stringContainsInOrder("\"\"")));
assertThat(encoded, not(stringContainsInOrder("null")));
}
/**
* #480
*/
@Test
public void testParseEmptyValue() {
String input = "{\"resourceType\":\"QuestionnaireResponse\",\"id\":\"123\",\"authored\":\"\",\"item\":[{\"item\":[{\"linkId\":\"\"}]}]}";
QuestionnaireResponse qr = ourCtx.newJsonParser().parseResource(QuestionnaireResponse.class, input);
assertEquals("QuestionnaireResponse/123", qr.getIdElement().getValue());
assertEquals(null, qr.getAuthored());
assertEquals(null, qr.getAuthoredElement().getValue());
assertEquals(null, qr.getAuthoredElement().getValueAsString());
assertEquals(null, qr.getItemFirstRep().getItemFirstRep().getLinkId());
assertEquals(null, qr.getItemFirstRep().getItemFirstRep().getLinkIdElement().getValue());
}
/**
* See #477
*/

View File

@ -36,6 +36,14 @@
has become unmaintained and has unfixed bugs so
it is no longer recommended for use.
</action>
<action type="fix" issue="480">
Make the parser configurable so that when
parsing an invalid empty value (e.g.
<![CDATA[<code>{"status":""}</code>]]>) the
parser will either throw a meaningful exception
or log a warning depending on the configured
error handler.
</action>
</release>
<release version="2.1" date="2016-11-11">
<action type="add">