* The {@link #invalidValue(ca.uhn.fhir.parser.IParserErrorHandler.IParseLocation, String, String)} * method will throw a {@link DataFormatException} by default since ignoring this type of error @@ -36,12 +36,20 @@ import static org.apache.commons.lang3.StringUtils.isBlank; * * @see IParser#setParserErrorHandler(IParserErrorHandler) * @see FhirContext#setParserErrorHandler(IParserErrorHandler) + * + *
+ * The {@link #extensionContainsValueAndNestedExtensions(ca.uhn.fhir.parser.IParserErrorHandler.IParseLocation)} + * method will throw a {@link DataFormatException} by default since ignoring this type of error will allow malformed + * resouces to be created and result in errors when attempts to read, update or delete the resource in the future. + * See {@link #setErrorOnInvalidExtension(boolean)} for information on this. + *
*/ public class LenientErrorHandler extends ParseErrorHandler implements IParserErrorHandler { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(LenientErrorHandler.class); private static final StrictErrorHandler STRICT_ERROR_HANDLER = new StrictErrorHandler(); private boolean myErrorOnInvalidValue = true; + private boolean myErrorOnInvalidExtension = true; private boolean myLogErrors; /** @@ -92,7 +100,7 @@ public class LenientErrorHandler extends ParseErrorHandler implements IParserErr /** * If set tofalse (default is true) invalid values will be logged. By
- * default invalid attribute values cause this error handler to throw a {@link DataFormatException} (unlike
+ * default, invalid attribute values cause this error handler to throw a {@link DataFormatException} (unlike
* other methods in this class which default to simply logging errors).
*
* Note that empty values (e.g. "") will not lead to an error when this is set to
@@ -105,6 +113,17 @@ public class LenientErrorHandler extends ParseErrorHandler implements IParserErr
return myErrorOnInvalidValue;
}
+ /**
+ * If set to false (default is true) invalid extensions will be logged. By
+ * default, invalid resource extensions cause this error handler to throw a {@link DataFormatException} (unlike
+ * other methods in this class which default to simply logging errors).
+ *
+ * @see #setErrorOnInvalidExtension(boolean)
+ */
+ public boolean isErrorOnInvalidExtension() {
+ return myErrorOnInvalidExtension;
+ }
+
@Override
public void missingRequiredElement(IParseLocation theLocation, String theElementName) {
if (myLogErrors) {
@@ -114,7 +133,7 @@ public class LenientErrorHandler extends ParseErrorHandler implements IParserErr
/**
* If set to false (default is true) invalid values will be logged. By
- * default invalid attribute values cause this error handler to throw a {@link DataFormatException} (unlike
+ * default, invalid attribute values cause this error handler to throw a {@link DataFormatException} (unlike
* other methods in this class which default to simply logging errors).
*
* Note that empty values (e.g. "") will not lead to an error when this is set to
@@ -129,6 +148,28 @@ public class LenientErrorHandler extends ParseErrorHandler implements IParserErr
return this;
}
+ /**
+ * If set to false (default is true) invalid extensions will be logged. By
+ * default, invalid resource extensions cause this error handler to throw a {@link DataFormatException} (unlike
+ * other methods in this class which default to simply logging errors).
+ *
+ * @return Returns a reference to this for easy method chaining
+ * @see #isErrorOnInvalidExtension()
+ */
+ public LenientErrorHandler setErrorOnInvalidExtension(boolean theErrorOnInvalidExtension) {
+ myErrorOnInvalidExtension = theErrorOnInvalidExtension;
+ return this;
+ }
+
+ /**
+ * If this method is called, both invalid resource extensions and invalid attribute values will set to simply logging errors.
+ */
+ public LenientErrorHandler disableAllErrors() {
+ myErrorOnInvalidValue = false;
+ myErrorOnInvalidExtension = false;
+ return this;
+ }
+
@Override
public void unexpectedRepeatingElement(IParseLocation theLocation, String theElementName) {
if (myLogErrors) {
@@ -158,8 +199,10 @@ public class LenientErrorHandler extends ParseErrorHandler implements IParserErr
}
@Override
- public void extensionContainsValueAndNestedExtensions(IParseLocation theLocation) {
- if (myLogErrors) {
+ public void extensionContainsValueAndNestedExtensions(IParseLocation theLocation){
+ if (myErrorOnInvalidExtension) {
+ STRICT_ERROR_HANDLER.extensionContainsValueAndNestedExtensions(theLocation);
+ } else if (myLogErrors) {
ourLog.warn("{}Extension contains both a value and nested extensions", describeLocation(theLocation));
}
}
diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4746-hl7v2-inbound-endpoint-should-reject-malformed-resources.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4746-hl7v2-inbound-endpoint-should-reject-malformed-resources.yaml
new file mode 100644
index 00000000000..328ccd606b3
--- /dev/null
+++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4746-hl7v2-inbound-endpoint-should-reject-malformed-resources.yaml
@@ -0,0 +1,7 @@
+---
+type: fix
+issue: 4746
+jira: SMILE-4710
+title: "Previously, the Lenient error handler will accept invalid extension containing value and nested extensions by default.
+This will lead to errors when client further attempt to read, update or delete the resources. Now, this has been fixed and
+a DataFormatException will be thrown instead."
diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java
index ca51ff69f8b..3d3165f736a 100644
--- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java
+++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java
@@ -76,7 +76,7 @@ import static org.apache.commons.lang3.StringUtils.defaultString;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
public class JpaStorageResourceParser implements IJpaStorageResourceParser {
- public static final LenientErrorHandler LENIENT_ERROR_HANDLER = new LenientErrorHandler(false).setErrorOnInvalidValue(false);
+ public static final LenientErrorHandler LENIENT_ERROR_HANDLER = new LenientErrorHandler(false).disableAllErrors();
private static final Logger ourLog = LoggerFactory.getLogger(JpaStorageResourceParser.class);
@Autowired
private FhirContext myFhirContext;
diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TolerantJsonParser.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TolerantJsonParser.java
index 09c475b4fe5..ec95f9d9861 100644
--- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TolerantJsonParser.java
+++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TolerantJsonParser.java
@@ -110,7 +110,7 @@ public class TolerantJsonParser extends JsonParser {
}
public static TolerantJsonParser createWithLenientErrorHandling(FhirContext theContext, @Nullable Long theResourcePid) {
- LenientErrorHandler errorHandler = new LenientErrorHandler(false).setErrorOnInvalidValue(false);
+ LenientErrorHandler errorHandler = new LenientErrorHandler(false).disableAllErrors();
return new TolerantJsonParser(theContext, errorHandler, theResourcePid);
}
}
diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/ErrorHandlerTest.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/ErrorHandlerTest.java
index 5980237ec95..54c94f7add0 100644
--- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/ErrorHandlerTest.java
+++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/ErrorHandlerTest.java
@@ -50,6 +50,7 @@ public class ErrorHandlerTest {
new LenientErrorHandler().unknownReference(null, null);
new LenientErrorHandler().incorrectJsonType(null, null, ValueType.ARRAY, null, ValueType.SCALAR, null);
new LenientErrorHandler().setErrorOnInvalidValue(false).invalidValue(null, "FOO", "");
+ new LenientErrorHandler().setErrorOnInvalidExtension(false).extensionContainsValueAndNestedExtensions(null);
new LenientErrorHandler().invalidValue(null, null, "");
try {
new LenientErrorHandler().invalidValue(null, "FOO", "");
@@ -57,6 +58,13 @@ public class ErrorHandlerTest {
} catch (DataFormatException e) {
// good, this one method defaults to causing an error
}
+
+ try {
+ new LenientErrorHandler().extensionContainsValueAndNestedExtensions(null);
+ fail();
+ } catch (DataFormatException e) {
+ // good, this one method defaults to causing an error
+ }
}
@Test
diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java
index b35ffafdb11..f05232ba97a 100644
--- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java
+++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java
@@ -1619,7 +1619,7 @@ public class XmlParserDstu3Test {
child.setValue(new StringType("CHILD_VALUE"));
// Lenient error handler
- IParser parser = ourCtx.newXmlParser();
+ IParser parser = ourCtx.newXmlParser().setParserErrorHandler(new LenientErrorHandler(false).disableAllErrors());
String output = parser.encodeResourceToString(p);
ourLog.info("Output: {}", output);
assertThat(output, containsString("http://root"));
diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java
index 26ad94eb604..9b716d0b7b7 100644
--- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java
+++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java
@@ -473,7 +473,6 @@ public class JsonParserR4Test extends BaseTest {
}
-
@Test
public void testEncodeWithInvalidExtensionContainingValueAndNestedExtensions() {
@@ -485,14 +484,15 @@ public class JsonParserR4Test extends BaseTest {
child.setUrl("http://child");
child.setValue(new StringType("CHILD_VALUE"));
+ // According to issue4129, all error handlers should reject malformed resources
// Lenient error handler
IParser parser = ourCtx.newJsonParser();
- String output = parser.encodeResourceToString(p);
- ourLog.info("Output: {}", output);
- assertThat(output, containsString("http://root"));
- assertThat(output, containsString("ROOT_VALUE"));
- assertThat(output, containsString("http://child"));
- assertThat(output, containsString("CHILD_VALUE"));
+ try {
+ parser.encodeResourceToString(p);
+ fail();
+ } catch (DataFormatException e) {
+ assertEquals(Msg.code(1827) + "[element=\"Patient(res).extension\"] Extension contains both a value and nested extensions", e.getMessage());
+ }
// Strict error handler
try {
@@ -505,6 +505,30 @@ public class JsonParserR4Test extends BaseTest {
}
+ @Test
+ public void testEncodeWithInvalidExtensionContainingValueAndNestedExtensions_withDisableAllErrorsShouldSucceed() {
+
+ Patient p = new Patient();
+ Extension root = p.addExtension();
+ root.setUrl("http://root");
+ root.setValue(new StringType("ROOT_VALUE"));
+ Extension child = root.addExtension();
+ child.setUrl("http://child");
+ child.setValue(new StringType("CHILD_VALUE"));
+
+ // Lenient error handler - should parse successfully with no error
+ LenientErrorHandler errorHandler = new LenientErrorHandler(true).disableAllErrors();
+ IParser parser = ourCtx.newJsonParser().setParserErrorHandler(errorHandler);
+ String output = parser.encodeResourceToString(p);
+ ourLog.info("Output: {}", output);
+ assertThat(output, containsString("http://root"));
+ assertThat(output, containsString("ROOT_VALUE"));
+ assertThat(output, containsString("http://child"));
+ assertThat(output, containsString("CHILD_VALUE"));
+ assertEquals(false, errorHandler.isErrorOnInvalidExtension());
+ assertEquals(false, errorHandler.isErrorOnInvalidValue());
+ }
+
@Test
public void testEncodeResourceWithMixedManualAndAutomaticContainedResourcesLocalFirst() {