Merge pull request #172 from hapifhir/base64_fix

Fix for NPEs in Base64BinaryType. Added tests to cover the cases.

org.hl7.fhir.dstu2/pom.xml

@@ -95,6 +95,12 @@
 			<artifactId>Saxon-HE</artifactId>
 			<scope>test</scope>
 		</dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter</artifactId>
+            <version>RELEASE</version>
+            <scope>test</scope>
+        </dependency>
 
     </dependencies>
 

org.hl7.fhir.dstu2/src/main/java/org/hl7/fhir/dstu2/model/Base64BinaryType.java

@@ -49,6 +49,7 @@ package org.hl7.fhir.dstu2.model;
  */
 
 
+import ca.uhn.fhir.parser.DataFormatException;
 import org.apache.commons.codec.binary.Base64;
 import ca.uhn.fhir.model.api.annotation.DatatypeDef;
 
@@ -74,11 +75,17 @@ public class Base64BinaryType extends PrimitiveType<byte[]> {
 
 	public Base64BinaryType(String theValue) {
 		super();
+    // Null values still result in non-null instance being created
+    if (theValue != null) checkValidBase64(theValue);
 		setValueAsString(theValue);
 	}
 
   protected byte[] parse(String theValue) {
+    if (theValue != null) {
       return Base64.decodeBase64(theValue.getBytes(ca.uhn.fhir.rest.api.Constants.CHARSET_UTF8));
+    } else {
+      return null;
+    }
   }
 
   protected String encode(byte[] theValue) {
@@ -96,4 +103,17 @@ public class Base64BinaryType extends PrimitiveType<byte[]> {
 	public String fhirType() {
 		return "base64Binary";
 	}
+
+  /**
+   * Checks if the passed in String is a valid {@link Base64} encoded String. Will throw a {@link DataFormatException} if not
+   * formatted correctly.
+   *
+   * @param toCheck {@link String} to check if valid {@link Base64}
+   * @throws DataFormatException
+   */
+  public void checkValidBase64(String toCheck) throws DataFormatException {
+    if (!Base64.isBase64(toCheck.getBytes())) {
+      throw new DataFormatException("");
+    }
+  }
 }

org.hl7.fhir.dstu2/src/test/java/org/hl7/fhir/dstu2/model/Base64BinaryTypeTest.java

@@ -0,0 +1,38 @@
+package org.hl7.fhir.dstu2.model;
+
+import ca.uhn.fhir.parser.DataFormatException;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+class Base64BinaryTypeTest {
+
+  @Test
+  @DisplayName("Passing a non Base64 encoded String to constructor causes exception.")
+  public void testNonBase64String() {
+    String nonBase64 = "Picard was the best starship captain.";
+    assertThrows(DataFormatException.class, () -> new Base64BinaryType(nonBase64));
+  }
+
+  @Test
+  @DisplayName("Null String value creates non-null instance with null value.")
+  public void testNullInstance() throws DataFormatException {
+    String v = null;
+    Base64BinaryType b64 = new Base64BinaryType(v);
+    Assertions.assertNotNull(b64);
+    Assertions.assertNull(b64.getValue());
+    Assertions.assertNull(b64.getValueAsString());
+  }
+
+  @Test
+  @DisplayName("Valid Base64 String creates non-null instance with non-null values.")
+  public void testValid() {
+    String v = "dGhpcyBpcyB2YWxpZCBiYXNlNjQ=";
+    Base64BinaryType b64 = new Base64BinaryType(v);
+    Assertions.assertNotNull(b64);
+    Assertions.assertNotNull(b64.getValue());
+    Assertions.assertEquals(v, b64.asStringValue());
+  }
+}

org.hl7.fhir.dstu2016may/pom.xml

@@ -109,6 +109,12 @@
 			<artifactId>Saxon-HE</artifactId>
 			<scope>test</scope>
 		</dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter</artifactId>
+            <version>RELEASE</version>
+            <scope>test</scope>
+        </dependency>
 
     </dependencies>
 

org.hl7.fhir.dstu2016may/src/main/java/org/hl7/fhir/dstu2016may/model/Base64BinaryType.java

@@ -49,6 +49,7 @@ package org.hl7.fhir.dstu2016may.model;
  */
 
 
+import ca.uhn.fhir.parser.DataFormatException;
 import org.apache.commons.codec.binary.Base64;
 
 import ca.uhn.fhir.model.api.annotation.DatatypeDef;
@@ -75,11 +76,17 @@ public class Base64BinaryType extends PrimitiveType<byte[]> {
 
 	public Base64BinaryType(String theValue) {
 		super();
+    // Null values still result in non-null instance being created
+    if (theValue != null) checkValidBase64(theValue);
 		setValueAsString(theValue);
 	}
 
   protected byte[] parse(String theValue) {
+    if (theValue != null) {
       return Base64.decodeBase64(theValue.getBytes(ca.uhn.fhir.rest.api.Constants.CHARSET_UTF8));
+    } else {
+      return null;
+    }
   }
 
   protected String encode(byte[] theValue) {
@@ -97,4 +104,17 @@ public class Base64BinaryType extends PrimitiveType<byte[]> {
 	public String fhirType() {
 		return "base64Binary";
 	}
+
+  /**
+   * Checks if the passed in String is a valid {@link Base64} encoded String. Will throw a {@link DataFormatException} if not
+   * formatted correctly.
+   *
+   * @param toCheck {@link String} to check if valid {@link Base64}
+   * @throws DataFormatException
+   */
+  public void checkValidBase64(String toCheck) throws DataFormatException {
+    if (!Base64.isBase64(toCheck.getBytes())) {
+      throw new DataFormatException("");
+    }
+  }
 }

org.hl7.fhir.dstu2016may/src/test/java/org/hl7/fhir/dstu2016may/model/Base64BinaryTypeTest.java

@@ -0,0 +1,38 @@
+package org.hl7.fhir.dstu2016may.model;
+
+import ca.uhn.fhir.parser.DataFormatException;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+class Base64BinaryTypeTest {
+
+  @Test
+  @DisplayName("Passing a non Base64 encoded String to constructor causes exception.")
+  public void testNonBase64String() {
+    String nonBase64 = "Picard was the best starship captain.";
+    assertThrows(DataFormatException.class, () -> new Base64BinaryType(nonBase64));
+  }
+
+  @Test
+  @DisplayName("Null String value creates non-null instance with null value.")
+  public void testNullInstance() throws DataFormatException {
+    String v = null;
+    Base64BinaryType b64 = new Base64BinaryType(v);
+    Assertions.assertNotNull(b64);
+    Assertions.assertNull(b64.getValue());
+    Assertions.assertNull(b64.getValueAsString());
+  }
+
+  @Test
+  @DisplayName("Valid Base64 String creates non-null instance with non-null values.")
+  public void testValid() {
+    String v = "dGhpcyBpcyB2YWxpZCBiYXNlNjQ=";
+    Base64BinaryType b64 = new Base64BinaryType(v);
+    Assertions.assertNotNull(b64);
+    Assertions.assertNotNull(b64.getValue());
+    Assertions.assertEquals(v, b64.asStringValue());
+  }
+}

org.hl7.fhir.dstu3/pom.xml

@@ -95,6 +95,12 @@
             <artifactId>Saxon-HE</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter</artifactId>
+            <version>RELEASE</version>
+            <scope>test</scope>
+        </dependency>
 
     </dependencies>
 

org.hl7.fhir.dstu3/src/main/java/org/hl7/fhir/dstu3/model/Base64BinaryType.java

@@ -51,6 +51,7 @@ package org.hl7.fhir.dstu3.model;
 
 import ca.uhn.fhir.model.api.IElement;
 import ca.uhn.fhir.model.api.annotation.DatatypeDef;
+import ca.uhn.fhir.parser.DataFormatException;
 import org.apache.commons.codec.binary.Base64;
 import org.hl7.fhir.instance.model.api.IBaseHasExtensions;
 import org.hl7.fhir.instance.model.api.IPrimitiveType;
@@ -83,11 +84,17 @@ public class Base64BinaryType extends PrimitiveType<byte[]> implements IPrimitiv
 
   public Base64BinaryType(String theValue) {
     super();
+    // Null values still result in non-null instance being created
+    if (theValue != null) checkValidBase64(theValue);
     setValueAsString(theValue);
   }
 
   protected byte[] parse(String theValue) {
+    if (theValue != null) {
       return Base64.decodeBase64(theValue.getBytes(ca.uhn.fhir.rest.api.Constants.CHARSET_UTF8));
+    } else {
+      return null;
+    }
   }
 
   protected String encode(byte[] theValue) {
@@ -128,6 +135,7 @@ public class Base64BinaryType extends PrimitiveType<byte[]> implements IPrimitiv
 
   @Override
   public void setValueAsString(String theValue) throws IllegalArgumentException {
+    fromStringValue(theValue);
     setValue(parse(theValue));
   }
 
@@ -153,5 +161,16 @@ public class Base64BinaryType extends PrimitiveType<byte[]> implements IPrimitiv
     return ca.uhn.fhir.util.ElementUtil.isEmpty(id, extension) && !hasValue();
   }
 
-
+  /**
+   * Checks if the passed in String is a valid {@link Base64} encoded String. Will throw a {@link DataFormatException} if not
+   * formatted correctly.
+   *
+   * @param toCheck {@link String} to check if valid {@link Base64}
+   * @throws DataFormatException
+   */
+  public void checkValidBase64(String toCheck) throws DataFormatException {
+    if (!Base64.isBase64(toCheck.getBytes())) {
+      throw new DataFormatException("");
+    }
+  }
 }

org.hl7.fhir.dstu3/src/test/java/org/hl7/fhir/dstu3/model/Base64BinaryTypeTest.java

@@ -0,0 +1,38 @@
+package org.hl7.fhir.dstu3.model;
+
+import ca.uhn.fhir.parser.DataFormatException;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+class Base64BinaryTypeTest {
+
+  @Test
+  @DisplayName("Passing a non Base64 encoded String to constructor causes exception.")
+  public void testNonBase64String() {
+    String nonBase64 = "Picard was the best starship captain.";
+    assertThrows(DataFormatException.class, () -> new Base64BinaryType(nonBase64));
+  }
+
+  @Test
+  @DisplayName("Null String value creates non-null instance with null value.")
+  public void testNullInstance() throws DataFormatException {
+    String v = null;
+    Base64BinaryType b64 = new Base64BinaryType(v);
+    Assertions.assertNotNull(b64);
+    Assertions.assertNull(b64.getValue());
+    Assertions.assertNull(b64.getValueAsString());
+  }
+
+  @Test
+  @DisplayName("Valid Base64 String creates non-null instance with non-null values.")
+  public void testValid() {
+    String v = "dGhpcyBpcyB2YWxpZCBiYXNlNjQ=";
+    Base64BinaryType b64 = new Base64BinaryType(v);
+    Assertions.assertNotNull(b64);
+    Assertions.assertNotNull(b64.getValue());
+    Assertions.assertEquals(v, b64.asStringValue());
+  }
+}

org.hl7.fhir.r4/pom.xml

@@ -101,6 +101,12 @@
 			<artifactId>Saxon-HE</artifactId>
 			<scope>test</scope>
 		</dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter</artifactId>
+            <version>RELEASE</version>
+            <scope>test</scope>
+        </dependency>
 
 	</dependencies>
 

org.hl7.fhir.r4/src/main/java/org/hl7/fhir/r4/model/Base64BinaryType.java

@@ -50,6 +50,7 @@ package org.hl7.fhir.r4.model;
 
 
 import ca.uhn.fhir.model.api.IElement;
+import ca.uhn.fhir.parser.DataFormatException;
 import org.apache.commons.codec.binary.Base64;
 
 import ca.uhn.fhir.model.api.annotation.DatatypeDef;
@@ -84,11 +85,17 @@ public class Base64BinaryType extends PrimitiveType<byte[]> implements IPrimitiv
 
   public Base64BinaryType(String theValue) {
     super();
+    // Null values still result in non-null instance being created
+    if (theValue != null) checkValidBase64(theValue);
     setValueAsString(theValue);
   }
 
   protected byte[] parse(String theValue) {
+    if (theValue != null) {
       return Base64.decodeBase64(theValue.getBytes(ca.uhn.fhir.rest.api.Constants.CHARSET_UTF8));
+    } else {
+      return null;
+    }
   }
 
   protected String encode(byte[] theValue) {
@@ -129,6 +136,7 @@ public class Base64BinaryType extends PrimitiveType<byte[]> implements IPrimitiv
 
   @Override
   public void setValueAsString(String theValue) throws IllegalArgumentException {
+    fromStringValue(theValue);
     setValue(parse(theValue));
   }
 
@@ -154,4 +162,16 @@ public class Base64BinaryType extends PrimitiveType<byte[]> implements IPrimitiv
     return ca.uhn.fhir.util.ElementUtil.isEmpty(id, extension) && !hasValue();
   }
 
+  /**
+   * Checks if the passed in String is a valid {@link Base64} encoded String. Will throw a {@link DataFormatException} if not
+   * formatted correctly.
+   *
+   * @param toCheck {@link String} to check if valid {@link Base64}
+   * @throws DataFormatException
+   */
+  public void checkValidBase64(String toCheck) throws DataFormatException {
+    if (!Base64.isBase64(toCheck.getBytes())) {
+      throw new DataFormatException("");
+    }
+  }
 }

org.hl7.fhir.r4/src/test/java/org/hl7/fhir/r4/model/Base64BinaryTypeTest.java

@@ -0,0 +1,38 @@
+package org.hl7.fhir.r4.model;
+
+import ca.uhn.fhir.parser.DataFormatException;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+class Base64BinaryTypeTest {
+
+  @Test
+  @DisplayName("Passing a non Base64 encoded String to constructor causes exception.")
+  public void testNonBase64String() {
+    String nonBase64 = "Picard was the best starship captain.";
+    assertThrows(DataFormatException.class, () -> new Base64BinaryType(nonBase64));
+  }
+
+  @Test
+  @DisplayName("Null String value creates non-null instance with null value.")
+  public void testNullInstance() throws DataFormatException {
+    String v = null;
+    Base64BinaryType b64 = new Base64BinaryType(v);
+    Assertions.assertNotNull(b64);
+    Assertions.assertNull(b64.getValue());
+    Assertions.assertNull(b64.getValueAsString());
+  }
+
+  @Test
+  @DisplayName("Valid Base64 String creates non-null instance with non-null values.")
+  public void testValid() {
+    String v = "dGhpcyBpcyB2YWxpZCBiYXNlNjQ=";
+    Base64BinaryType b64 = new Base64BinaryType(v);
+    Assertions.assertNotNull(b64);
+    Assertions.assertNotNull(b64.getValue());
+    Assertions.assertEquals(v, b64.asStringValue());
+  }
+}

org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/formats/JsonParser.java

@@ -54,6 +54,7 @@ package org.hl7.fhir.r5.formats;
 
 
 
+import ca.uhn.fhir.parser.DataFormatException;
 import org.hl7.fhir.r5.model.*;
 import org.hl7.fhir.utilities.Utilities;
 import org.hl7.fhir.utilities.xhtml.XhtmlNode;
@@ -160,8 +161,7 @@ public class JsonParser extends JsonParserBase {
   }
 
   protected Base64BinaryType parseBase64Binary(String v) throws IOException, FHIRFormatError {
-    Base64BinaryType res = new Base64BinaryType(v);
+      return new Base64BinaryType(v);
-    return res;
   }
 
   protected UnsignedIntType parseUnsignedInt(String v) throws IOException, FHIRFormatError {

org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/model/Base64BinaryType.java

@@ -51,8 +51,8 @@ package org.hl7.fhir.r5.model;
 
 import ca.uhn.fhir.model.api.IElement;
 import ca.uhn.fhir.model.api.annotation.DatatypeDef;
+import ca.uhn.fhir.parser.DataFormatException;
 import org.apache.commons.codec.binary.Base64;
-import org.apache.commons.lang3.StringUtils;
 import org.hl7.fhir.instance.model.api.IBaseHasExtensions;
 import org.hl7.fhir.instance.model.api.IPrimitiveType;
 
@@ -84,11 +84,17 @@ public class Base64BinaryType extends PrimitiveType<byte[]> implements IPrimitiv
 
   public Base64BinaryType(String theValue) {
     super();
+    // Null values still result in non-null instance being created
+    if (theValue != null) checkValidBase64(theValue);
     setValueAsString(theValue);
   }
 
   protected byte[] parse(String theValue) {
+    if (theValue != null) {
       return Base64.decodeBase64(theValue.getBytes(ca.uhn.fhir.rest.api.Constants.CHARSET_UTF8));
+    } else {
+      return null;
+    }
   }
 
   protected String encode(byte[] theValue) {
@@ -129,6 +135,7 @@ public class Base64BinaryType extends PrimitiveType<byte[]> implements IPrimitiv
 
   @Override
   public void setValueAsString(String theValue) throws IllegalArgumentException {
+    fromStringValue(theValue);
     setValue(parse(theValue));
   }
 
@@ -158,4 +165,17 @@ public class Base64BinaryType extends PrimitiveType<byte[]> implements IPrimitiv
   public String primitiveValue() {
     return encode(myValue);
   }
+
+  /**
+   * Checks if the passed in String is a valid {@link Base64} encoded String. Will throw a {@link DataFormatException} if not
+   * formatted correctly.
+   *
+   * @param toCheck {@link String} to check if valid {@link Base64}
+   * @throws DataFormatException
+   */
+  public void checkValidBase64(String toCheck) throws DataFormatException {
+    if (!Base64.isBase64(toCheck.getBytes())) {
+      throw new DataFormatException("");
+    }
+  }
 }

org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/model/Base64BinaryTypeTest.java

@@ -0,0 +1,39 @@
+package org.hl7.fhir.r5.model;
+
+import ca.uhn.fhir.parser.DataFormatException;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+class Base64BinaryTypeTest {
+
+  @Test
+  @DisplayName("Passing a non Base64 encoded String to constructor causes exception.")
+  public void testNonBase64String() {
+    String nonBase64 = "Picard was the best starship captain.";
+    assertThrows(DataFormatException.class, () -> new Base64BinaryType(nonBase64));
+  }
+
+  @Test
+  @DisplayName("Null String value creates non-null instance with null value.")
+  public void testNullInstance() throws DataFormatException {
+    String v = null;
+    Base64BinaryType b64 = new Base64BinaryType(v);
+    Assertions.assertNotNull(b64);
+    Assertions.assertNull(b64.getValue());
+    Assertions.assertNull(b64.getValueAsString());
+  }
+
+  @Test
+  @DisplayName("Valid Base64 String creates non-null instance with non-null values.")
+  public void testValid() {
+    String v = "dGhpcyBpcyB2YWxpZCBiYXNlNjQ=";
+    Base64BinaryType b64 = new Base64BinaryType(v);
+    Assertions.assertNotNull(b64);
+    Assertions.assertNotNull(b64.getValue());
+    Assertions.assertEquals(v, b64.asStringValue());
+  }
+
+}