Don't crash if resrouce references being parsed have unexpected child

elements

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

@@ -227,7 +227,7 @@ class ParserState<T> {
 
 	/**
 	 * @param theResourceType
-	 *            May be null
+	 *           May be null
 	 */
 	static <T extends IBaseResource> ParserState<T> getPreResourceInstance(Class<T> theResourceType, FhirContext theContext, boolean theJsonMode, IParserErrorHandler theErrorHandler)
 			throws DataFormatException {
@@ -796,6 +796,10 @@ class ParserState<T> {
 			myPreResourceState = thePreResourceState;
 		}
 
+		/**
+		 * @param theValue
+		 *           The attribute value
+		 */
 		public void attributeValue(String theName, String theValue) throws DataFormatException {
 			myErrorHandler.unknownAttribute(null, theName);
 		}
@@ -804,6 +808,15 @@ class ParserState<T> {
 			// ignore by default
 		}
 
+		protected void logAndSwallowUnexpectedElement(String theLocalPart) {
+			myErrorHandler.unknownElement(null, theLocalPart);
+			push(new SwallowChildrenWholeState(getPreResourceState()));
+		}
+
+		/**
+		 * @param theNamespaceUri
+		 *           The XML namespace (if XML) or null
+		 */
 		public void enteringNewElement(String theNamespaceUri, String theLocalPart) throws DataFormatException {
 			myErrorHandler.unknownElement(null, theLocalPart);
 		}
@@ -860,7 +873,7 @@ class ParserState<T> {
 
 		/**
 		 * @param theData
-		 *            The string value
+		 *           The string value
 		 */
 		public void string(String theData) {
 			// ignore by default
@@ -872,7 +885,7 @@ class ParserState<T> {
 
 		/**
 		 * @param theNextEvent
-		 *            The XML event
+		 *           The XML event
 		 */
 		public void xmlEvent(XMLEvent theNextEvent) {
 			// ignore
@@ -1143,13 +1156,13 @@ class ParserState<T> {
 			} else if ("url".equals(theLocalPart)) {
 				push(new PrimitiveState(getPreResourceState(), myEntry.getLinkSearch()));
 			} else {
-				ourLog.warn("Unexpected element in Bundle.entry.search: " + theLocalPart);
+				logAndSwallowUnexpectedElement(theLocalPart);
-				push(new SwallowChildrenWholeState(getPreResourceState()));
 			}
 		}
 
 	}
 
+
 	private class BundleLinkState extends BaseState {
 
 		private BundleEntry myEntry;
@@ -1553,13 +1566,13 @@ class ParserState<T> {
 				push(new SwallowChildrenWholeState(getPreResourceState()));
 				return;
 			}
-			
+
 			if ((child.getMax() == 0 || child.getMax() == 1) && !myParsedNonRepeatableNames.add(theChildName)) {
 				myErrorHandler.unexpectedRepeatingElement(null, theChildName);
 				push(new SwallowChildrenWholeState(getPreResourceState()));
 				return;
 			}
-			
+
 			BaseRuntimeElementDefinition<?> target = child.getChildByName(theChildName);
 			if (target == null) {
 				// This is a bug with the structures and shouldn't happen..
@@ -2328,11 +2341,14 @@ class ParserState<T> {
 				} else if ("resource".equals(theLocalPart)) {
 					mySubState = ResourceReferenceSubState.REFERENCE;
 					break;
+				} else {
+					logAndSwallowUnexpectedElement(theLocalPart);
+					break;
 				}
-				//$FALL-THROUGH$
 			case DISPLAY:
 			case REFERENCE:
-				throw new DataFormatException("Unexpected element: " + theLocalPart);
+				logAndSwallowUnexpectedElement(theLocalPart);
+				break;
 			}
 		}
 

hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/parser/XmlParserTest.java

@@ -108,39 +108,14 @@ public class XmlParserTest {
 		String str = ourCtx.newXmlParser().encodeResourceToString(patient);
 		ourLog.info(str);
 
-		assertThat(
+		assertThat(str, Matchers.stringContainsInOrder(
-				str,
+				"<Patient xmlns=\"http://hl7.org/fhir\"><extension url=\"http://foo/someOrg\"><valueResource><reference value=\"#1\"/></valueResource></extension><contained><Organization xmlns=\"http://hl7.org/fhir\" id=\"1\"><name value=\"OrgName\"/></Organization></contained><name><family value=\"PatientName\"/></name></Patient>"));
-				Matchers.stringContainsInOrder("<Patient xmlns=\"http://hl7.org/fhir\"><extension url=\"http://foo/someOrg\"><valueResource><reference value=\"#1\"/></valueResource></extension><contained><Organization xmlns=\"http://hl7.org/fhir\" id=\"1\"><name value=\"OrgName\"/></Organization></contained><name><family value=\"PatientName\"/></name></Patient>"));
 
 		MyPatient parse = ourCtx.newXmlParser().parseResource(MyPatient.class, str);
 		assertEquals("PatientName", parse.getNameFirstRep().getFamilyAsSingleString());
 		assertEquals("OrgName", ((MyOrganization) parse.getSomeOrganization().getResource()).getName().getValue());
 	}
 
-	
-	@Test
-	public void testParseAndEncodeHugeValue() {
-		int len = 1000000;
-		byte[] bytes = new byte[len];
-		for (int i = 0; i < len; i++) {
-			bytes[i] = (byte) (Math.random() * Byte.MAX_VALUE);
-		}
-		
-		AttachmentDt att = new AttachmentDt();
-		att.setData(bytes);
-		
-		Observation obs = new Observation();
-		obs.setValue(att);
-		
-		String str = ourCtx.newXmlParser().encodeResourceToString(obs);
-		assertThat(str.length(), Matchers.greaterThan(len));
-		
-		obs = ourCtx.newXmlParser().parseResource(Observation.class, str);
-		att = (AttachmentDt) obs.getValue();
-		assertArrayEquals(bytes, att.getData().getValue());
-	}
-	
-	
 	/**
 	 * Test for #82 - Not yet enabled because the test won't pass
 	 */
@@ -157,9 +132,8 @@ public class XmlParserTest {
 		String str = ourCtx.newXmlParser().encodeResourceToString(patient);
 		ourLog.info(str);
 
-		assertThat(
+		assertThat(str, Matchers.stringContainsInOrder(
-				str,
+				"<Patient xmlns=\"http://hl7.org/fhir\"><contained><Organization xmlns=\"http://hl7.org/fhir\" id=\"1\"><name value=\"OrgName\"/></Organization></contained><name><family value=\"PatientName\"/></name><managingOrganization><reference value=\"#1\"/></managingOrganization></Patient>"));
-				Matchers.stringContainsInOrder("<Patient xmlns=\"http://hl7.org/fhir\"><contained><Organization xmlns=\"http://hl7.org/fhir\" id=\"1\"><name value=\"OrgName\"/></Organization></contained><name><family value=\"PatientName\"/></name><managingOrganization><reference value=\"#1\"/></managingOrganization></Patient>"));
 
 		MyPatient parse = ourCtx.newXmlParser().parseResource(MyPatient.class, str);
 		assertEquals("PatientName", parse.getNameFirstRep().getFamilyAsSingleString());
@@ -251,13 +225,11 @@ public class XmlParserTest {
 		String enc = ourCtx.newXmlParser().encodeResourceToString(patient);
 		assertThat(enc, containsString("<Patient xmlns=\"http://hl7.org/fhir\"><extension url=\"http://example.com/extensions#someext\"><valueDateTime value=\"2011-01-02T11:13:15\"/></extension>"));
 		assertThat(enc, containsString("<modifierExtension url=\"http://example.com/extensions#modext\"><valueDate value=\"1995-01-02\"/></modifierExtension>"));
-		assertThat(
+		assertThat(enc, containsString(
-				enc,
+				"<extension url=\"http://example.com#parent\"><extension url=\"http://example.com#child\"><valueString value=\"value1\"/></extension><extension url=\"http://example.com#child\"><valueString value=\"value2\"/></extension></extension>"));
-				containsString("<extension url=\"http://example.com#parent\"><extension url=\"http://example.com#child\"><valueString value=\"value1\"/></extension><extension url=\"http://example.com#child\"><valueString value=\"value2\"/></extension></extension>"));
 		assertThat(enc, containsString("<given value=\"Joe\"><extension url=\"http://examples.com#givenext\"><valueString value=\"given\"/></extension></given>"));
-		assertThat(
+		assertThat(enc, containsString(
-				enc,
+				"<given value=\"Shmoe\"><extension url=\"http://examples.com#givenext_parent\"><extension url=\"http://examples.com#givenext_child\"><valueString value=\"CHILD\"/></extension></extension></given>"));
-				containsString("<given value=\"Shmoe\"><extension url=\"http://examples.com#givenext_parent\"><extension url=\"http://examples.com#givenext_child\"><valueString value=\"CHILD\"/></extension></extension></given>"));
 
 		/*
 		 * Now parse this back
@@ -306,9 +278,9 @@ public class XmlParserTest {
 		comp.addSection().getContent().setResource(new AllergyIntolerance().addIdentifier("foo", "bar"));
 		comp.addSection().getContent().setResource(new AllergyIntolerance().addIdentifier("foo", "bar"));
 		comp.addSection().getContent().setResource(new AllergyIntolerance().addIdentifier("foo", "bar"));
-		
+
 		IParser parser = ourCtx.newJsonParser().setPrettyPrint(true);
-		
+
 		String string = parser.encodeResourceToString(comp);
 		ourLog.info(string);
 
@@ -331,9 +303,9 @@ public class XmlParserTest {
 		comp.addSection().getContent().setResource(new AllergyIntolerance().addIdentifier("foo", "bar"));
 		comp.addSection().getContent().setResource(new AllergyIntolerance().addIdentifier("foo", "bar"));
 		comp.addSection().getContent().setResource(new AllergyIntolerance().addIdentifier("foo", "bar"));
-		
+
 		IParser parser = ourCtx.newXmlParser().setPrettyPrint(true);
-		
+
 		String string = parser.encodeResourceToString(comp);
 		ourLog.info(string);
 
@@ -500,28 +472,6 @@ public class XmlParserTest {
 
 	}
 
-	
-	@Test
-	public void testEncodeContainedWithSelfReference() {
-		IParser xmlParser = ourCtx.newXmlParser().setPrettyPrint(true);
-
-		// Create an organization, note that the organization does not have an ID
-		Organization org = new Organization();
-		org.getName().setValue("Contained Test Organization");
-		org.setPartOf(new ResourceReferenceDt(org));
-
-		// Create a patient
-		Patient patient = new Patient();
-		patient.getManagingOrganization().setResource(org);
-
-		String encoded = xmlParser.encodeResourceToString(patient);
-		ourLog.info(encoded);
-		assertThat(encoded, containsString("<contained>"));
-		assertThat(encoded, containsString("<reference value=\"#1\"/>"));
-	}
-	
-	
-	
 	@Test
 	public void testEncodeContained() {
 		IParser xmlParser = ourCtx.newXmlParser().setPrettyPrint(true);
@@ -673,6 +623,25 @@ public class XmlParserTest {
 
 	}
 
+	@Test
+	public void testEncodeContainedWithSelfReference() {
+		IParser xmlParser = ourCtx.newXmlParser().setPrettyPrint(true);
+
+		// Create an organization, note that the organization does not have an ID
+		Organization org = new Organization();
+		org.getName().setValue("Contained Test Organization");
+		org.setPartOf(new ResourceReferenceDt(org));
+
+		// Create a patient
+		Patient patient = new Patient();
+		patient.getManagingOrganization().setResource(org);
+
+		String encoded = xmlParser.encodeResourceToString(patient);
+		ourLog.info(encoded);
+		assertThat(encoded, containsString("<contained>"));
+		assertThat(encoded, containsString("<reference value=\"#1\"/>"));
+	}
+
 	@Test
 	public void testEncodeDeclaredExtensionWithAddressContent() {
 		IParser parser = ourCtx.newXmlParser();
@@ -1265,9 +1234,8 @@ public class XmlParserTest {
 
 		String enc = ourCtx.newXmlParser().encodeResourceToString(patient);
 		assertThat(enc, containsString("<Patient xmlns=\"http://hl7.org/fhir\"><extension url=\"http://example.com/extensions#someext\"><valueDateTime value=\"2011-01-02T11:13:15\"/></extension>"));
-		assertThat(
+		assertThat(enc, containsString(
-				enc,
+				"<extension url=\"http://example.com#parent\"><extension url=\"http://example.com#child\"><valueString value=\"value1\"/></extension><extension url=\"http://example.com#child\"><valueString value=\"value1\"/></extension></extension>"));
-				containsString("<extension url=\"http://example.com#parent\"><extension url=\"http://example.com#child\"><valueString value=\"value1\"/></extension><extension url=\"http://example.com#child\"><valueString value=\"value1\"/></extension></extension>"));
 		assertThat(enc, containsString("<given value=\"Joe\"><extension url=\"http://examples.com#givenext\"><valueString value=\"given\"/></extension></given>"));
 	}
 
@@ -1360,6 +1328,28 @@ public class XmlParserTest {
 
 	}
 
+	@Test
+	public void testParseAndEncodeHugeValue() {
+		int len = 1000000;
+		byte[] bytes = new byte[len];
+		for (int i = 0; i < len; i++) {
+			bytes[i] = (byte) (Math.random() * Byte.MAX_VALUE);
+		}
+
+		AttachmentDt att = new AttachmentDt();
+		att.setData(bytes);
+
+		Observation obs = new Observation();
+		obs.setValue(att);
+
+		String str = ourCtx.newXmlParser().encodeResourceToString(obs);
+		assertThat(str.length(), Matchers.greaterThan(len));
+
+		obs = ourCtx.newXmlParser().parseResource(Observation.class, str);
+		att = (AttachmentDt) obs.getValue();
+		assertArrayEquals(bytes, att.getData().getValue());
+	}
+
 	/**
 	 * See #131
 	 */
@@ -1685,6 +1675,20 @@ public class XmlParserTest {
 
 	}
 
+	@Test
+	public void testParseErrorHandlerDuplicateElement() {
+		String input = "<Patient><active value=\"true\"/><active value=\"false\"/></Patient>";
+		try {
+			ourCtx.newXmlParser().setParserErrorHandler(new StrictErrorHandler()).parseResource(Patient.class, input);
+			fail();
+		} catch (DataFormatException e) {
+			assertThat(e.getMessage(), containsString("Multiple repetitions"));
+		}
+
+		Patient p = ourCtx.newXmlParser().setParserErrorHandler(new LenientErrorHandler()).parseResource(Patient.class, input);
+		assertEquals("true", p.getActive().getValueAsString());
+	}
+
 	@Test
 	public void testParseErrorHandlerNoError() {
 		String input = "<Patient></Patient>";
@@ -1719,20 +1723,6 @@ public class XmlParserTest {
 		assertEquals(p.getName().get(0).getFamily().get(0).getValue(), "AAA");
 	}
 
-	@Test
-	public void testParseErrorHandlerDuplicateElement() {
-		String input = "<Patient><active value=\"true\"/><active value=\"false\"/></Patient>";
-		try {
-			ourCtx.newXmlParser().setParserErrorHandler(new StrictErrorHandler()).parseResource(Patient.class, input);
-			fail();
-		} catch (DataFormatException e) {
-			assertThat(e.getMessage(), containsString("Multiple repetitions"));
-		}
-
-		Patient p = ourCtx.newXmlParser().setParserErrorHandler(new LenientErrorHandler()).parseResource(Patient.class, input);
-		assertEquals("true", p.getActive().getValueAsString());
-	}
-
 	@Test
 	public void testParseFeedWithListResource() throws ConfigurationException, DataFormatException, IOException {
 
@@ -1781,18 +1771,35 @@ public class XmlParserTest {
 
 	}
 
+	@Test
+	public void testParseReferenceWithUnexpectedChild() {
+		//@formatter:off
+		String input = 
+				  "<Patient xmlns=\"http://hl7.org/fhir\">"
+				+ "  <managingOrganization>"
+				+ "    <reference value=\"Patient/123\"/>"
+				+ "    <text value=\"BLAH\"/>"
+				+ "  </managingOrganization>"
+				+ "</Patient>";
+		//@formatter:on
+
+		Patient parsed = ourCtx.newXmlParser().parseResource(Patient.class, input);
+		String output = ourCtx.newXmlParser().encodeResourceToString(parsed);
+
+		assertEquals("<Patient xmlns=\"http://hl7.org/fhir\"><managingOrganization><reference value=\"Patient/123\"/></managingOrganization></Patient>", output);
+	}
+
 	/**
 	 * #175
 	 */
-//	@Test
+	// @Test
 	public void testParseTextWithUnknownEntity() {
-		String msg = "<Patient xmlns=\"http://hl7.org/fhir\"><text><status value=\"generated\"/>"
+		String msg = "<Patient xmlns=\"http://hl7.org/fhir\"><text><status value=\"generated\"/>" + "<div xmlns=\"http://www.w3.org/1999/xhtml\">Trade &trade;</div></text></Patient>";
-				+ "<div xmlns=\"http://www.w3.org/1999/xhtml\">Trade &trade;</div></text></Patient>";
 		Patient pt = ourCtx.newXmlParser().parseResource(Patient.class, msg);
 
 		ourLog.info(pt.getText().getDiv().getValueAsString());
 		assertThat(pt.getText().getDiv().getValueAsString(), containsString("Trade ™"));
-		
+
 		String enc = ourCtx.newXmlParser().encodeResourceToString(pt);
 		ourLog.info(enc);
 		assertThat(enc, containsString("Trade ™"));

src/changes/changes.xml

@@ -72,6 +72,10 @@
 				a trailing comma or an escaped backslash. Thanks to GitHub user
 				@SherryH for all of her help in diagnosing this issue!
 			</action>
+			<action type="fix">
+				Avoid crash when parsing if an invalid child element is found in 
+				a resource reference.
+			</action>
 		</release>
 		<release version="1.1" date="2015-07-13">
 			<action type="add">