Skip to content

Commit

Permalink
Merge pull request #2820 from IBM/issue-2605
Browse files Browse the repository at this point in the history
Reject FHIR resources with control chars #2605
  • Loading branch information
prb112 authored Oct 12, 2021
2 parents 25e2888 + fd39c49 commit 37a0eb2
Show file tree
Hide file tree
Showing 13 changed files with 792 additions and 26 deletions.
5 changes: 4 additions & 1 deletion docs/src/pages/guides/FHIRServerUsersGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ layout: post
title: IBM FHIR Server User's Guide
description: IBM FHIR Server User's Guide
Copyright: years 2017, 2021
lastupdated: "2021-07-28"
lastupdated: "2021-10-12"
permalink: /FHIRServerUsersGuide/
---

Expand Down Expand Up @@ -1963,6 +1963,7 @@ This section contains reference information about each of the configuration prop
|`fhirServer/core/defaultHandling`|string|The default handling preference of the server (*strict* or *lenient*) which determines how the server handles unrecognized search parameters and resource elements.|
|`fhirServer/core/allowClientHandlingPref`|boolean|Indicates whether the client is allowed to override the server default handling preference using the `Prefer:handling` header value part.|
|`fhirServer/core/checkReferenceTypes`|boolean|Indicates whether reference type checking is performed by the server during parsing / deserialization.|
|`fhirServer/core/checkUnicodeControl`|boolean|Indicates whether Strings and Codes are checked for invalid Unicode control characters.|
|`fhirServer/core/serverRegistryResourceProviderEnabled`|boolean|Indicates whether the server registry resource provider should be used by the FHIR registry component to access definitional resources through the persistence layer.|
|`fhirServer/core/serverResolveFunctionEnabled`|boolean|Indicates whether the server resolve function should be used by the FHIRPath evaluator to resolve references through the persistence layer.|
|`fhirServer/core/conditionalDeleteMaxNumber`|integer|The maximum number of matches supported in conditional delete. |
Expand Down Expand Up @@ -2136,6 +2137,7 @@ This section contains reference information about each of the configuration prop
|`fhirServer/core/defaultHandling`|strict|
|`fhirServer/core/allowClientHandlingPref`|true|
|`fhirServer/core/checkReferenceTypes`|true|
|`fhirServer/core/checkUnicodeControl`|true|
|`fhirServer/core/serverRegistryResourceProviderEnabled`|false|
|`fhirServer/core/serverResolveFunctionEnabled`|false|
|`fhirServer/core/conditionalDeleteMaxNumber`|10|
Expand Down Expand Up @@ -2274,6 +2276,7 @@ must restart the server for that change to take effect.
|`fhirServer/core/defaultHandling`|Y|Y|
|`fhirServer/core/allowClientHandlingPref`|Y|Y|
|`fhirServer/core/checkReferenceTypes`|N|N|
|`fhirServer/core/checkUnicodeControl`|N|N|
|`fhirServer/core/serverRegistryResourceProviderEnabled`|N|N|
|`fhirServer/core/serverResolveFunctionEnabled`|N|N|
|`fhirServer/core/conditionalDeleteMaxNumber`|Y|Y|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public class FHIRConfiguration {
public static final String PROPERTY_DEFAULT_HANDLING = "fhirServer/core/defaultHandling";
public static final String PROPERTY_ALLOW_CLIENT_HANDLING_PREF = "fhirServer/core/allowClientHandlingPref";
public static final String PROPERTY_CHECK_REFERENCE_TYPES = "fhirServer/core/checkReferenceTypes";
public static final String PROPERTY_CHECK_UNICODE_CONTROL = "fhirServer/core/checkUnicodeControl";
public static final String PROPERTY_CONDITIONAL_DELETE_MAX_NUMBER = "fhirServer/core/conditionalDeleteMaxNumber";
public static final String PROPERTY_SERVER_REGISTRY_RESOURCE_PROVIDER_ENABLED = "fhirServer/core/serverRegistryResourceProviderEnabled";
public static final String PROPERTY_SERVER_RESOLVE_FUNCTION_ENABLED = "fhirServer/core/serverResolveFunctionEnabled";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2019
* (C) Copyright IBM Corp. 2019, 2021
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -33,7 +33,7 @@ public final class FHIRModelConfig {
* Used to determine whether the toString method return value should be formatted
*/
public static final String PROPERTY_TO_STRING_PRETTY_PRINTING = "com.ibm.fhir.model.toStringPrettyPrinting";

/**
* Used to determine whether reference types are checked during object construction
*/
Expand All @@ -46,88 +46,102 @@ public final class FHIRModelConfig {
*/
public static final String PROPERTY_EXTENDED_CODEABLE_CONCEPT_VALIDATION = "com.ibm.fhir.model.extendedCodeableConceptValidation";

/**
* Used to determine whether unicode control characters are checked
*/
public static final String PROPERTY_CHECK_UNICODE_CONTROL_CHARS = "com.ibm.fhir.model.checkUnicodeControl";

private static final Format DEFAULT_TO_STRING_FORMAT = Format.JSON;
private static final int DEFAULT_TO_STRING_INDENT_AMOUNT = 2;
private static final boolean DEFAULT_TO_STRING_PRETTY_PRINTING = true;
private static final boolean DEFAULT_CHECK_REFERENCE_TYPES = true;
private static final boolean DEFAULT_CHECK_UNICODE_CONTROL_CHARS = true;
private static final boolean DEFAULT_EXTENDED_CODEABLE_CONCEPT_VALIDATION = true;

private static final Map<String, Object> properties = new ConcurrentHashMap<>();

private FHIRModelConfig() { }

public static void setToStringFormat(Format format) {
setProperty(PROPERTY_TO_STRING_FORMAT, format);
}

public static Format getToStringFormat() {
return getPropertyOrDefault(PROPERTY_TO_STRING_FORMAT, DEFAULT_TO_STRING_FORMAT, Format.class);
}

public static void setToStringIndentAmount(int indentAmount) {
setProperty(PROPERTY_TO_STRING_INDENT_AMOUNT, indentAmount);
}

public static int getToStringIndentAmount() {
return getPropertyOrDefault(PROPERTY_TO_STRING_INDENT_AMOUNT, DEFAULT_TO_STRING_INDENT_AMOUNT, Integer.class);
}

public static void setToStringPrettyPrinting(boolean prettyPrinting) {
setProperty(PROPERTY_TO_STRING_PRETTY_PRINTING, prettyPrinting);
}

public static boolean getToStringPrettyPrinting() {
return getPropertyOrDefault(PROPERTY_TO_STRING_PRETTY_PRINTING, DEFAULT_TO_STRING_PRETTY_PRINTING, Boolean.class);
}

public static void setCheckReferenceTypes(boolean checkReferenceTypes) {
setProperty(PROPERTY_CHECK_REFERENCE_TYPES, checkReferenceTypes);
}

public static boolean getCheckReferenceTypes() {
return getPropertyOrDefault(PROPERTY_CHECK_REFERENCE_TYPES, DEFAULT_CHECK_REFERENCE_TYPES, Boolean.class);
}


public static void setCheckUnicodeControlChars(boolean checkUnicodeControlChars) {
setProperty(PROPERTY_CHECK_UNICODE_CONTROL_CHARS, checkUnicodeControlChars);
}

public static boolean shouldCheckUnicodeControlChars() {
return getPropertyOrDefault(PROPERTY_CHECK_UNICODE_CONTROL_CHARS, DEFAULT_CHECK_UNICODE_CONTROL_CHARS, Boolean.class);
}

public static void setExtendedCodeableConceptValidation(boolean extendedCodeableConceptValidation) {
setProperty(PROPERTY_EXTENDED_CODEABLE_CONCEPT_VALIDATION, extendedCodeableConceptValidation);
}

public static boolean getExtendedCodeableConceptValidation() {
return getPropertyOrDefault(PROPERTY_EXTENDED_CODEABLE_CONCEPT_VALIDATION, DEFAULT_EXTENDED_CODEABLE_CONCEPT_VALIDATION, Boolean.class);
}

public static void setProperty(String name, Object value) {
properties.put(requireNonNull(name), requireNonNull(value));
}

public static Object removeProperty(String name) {
return properties.remove(requireNonNull(name));
}

public static <T> T removeProperty(String name, Class<T> type) {
return requireNonNull(type).cast(removeProperty(name));
}

public static Object getProperty(String name) {
return properties.get(requireNonNull(name));
}

public static Object getPropertyOrDefault(String name, Object defaultValue) {
return properties.getOrDefault(requireNonNull(name), requireNonNull(defaultValue));
}

public static <T> T getProperty(String name, Class<T> type) {
return requireNonNull(type).cast(getProperty(name));
}

public static <T> T getPropertyOrDefault(String name, T defaultValue, Class<T> type) {
return requireNonNull(type).cast(getPropertyOrDefault(name, defaultValue));
}

public static Map<String, Object> getProperties() {
return Collections.unmodifiableMap(properties);
}

public static Set<String> getPropertyNames() {
return Collections.unmodifiableSet(properties.keySet());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ public Validator initialValue() {
}
};
private static final Set<Character> WHITESPACE = new HashSet<>(Arrays.asList(' ', '\t', '\r', '\n'));
private static final Set<Character> UNSUPPORTED_UNICODE = buildUnsupportedUnicodeCharacterSet();

private static final char [] BASE64_CHARS = {
'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P',
'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', 'a', 'b', 'c', 'd', 'e', 'f',
Expand All @@ -80,6 +82,20 @@ public Validator initialValue() {

private ValidationSupport() { }

/**
* Builds a set of unsupported unicode characters per the specification.
* @return
*/
private static Set<Character> buildUnsupportedUnicodeCharacterSet() {
Set<Character> chars = new HashSet<>();
for (int i = 0; i < 32; i++) {
if (i != 9 && i != 10 && i != 13) {
chars.add(Character.valueOf((char) i));
}
}
return chars;
}

private static Map<Character, Integer> buildBase64IndexMap() {
Map<Character, Integer> base64IndexMap = new LinkedHashMap<>();
for (int i = 0; i < BASE64_CHARS.length; i++) {
Expand Down Expand Up @@ -107,16 +123,56 @@ public static void checkString(String s) {
for (int i = 0; i < s.length(); i++) {
char ch = s.charAt(i);
if (!Character.isWhitespace(ch)) {
checkUnsupportedUnicode(s, ch);
count++;
} else if (!WHITESPACE.contains(ch)) {
throw new IllegalStateException(String.format("String value: '%s' is not valid with respect to pattern: [ \\r\\n\\t\\S]+", s));
throw new IllegalStateException(buildIllegalWhiteSpaceException(s));
}
}
if (count < MIN_STRING_LENGTH) {
throw new IllegalStateException(String.format("Trimmed String value length: %d is less than minimum required length: %d", count, MIN_STRING_LENGTH));
}
}

/**
* wraps the building of the illegal white space exception.
* @param s
* @return
*/
private static String buildIllegalWhiteSpaceException(String s) {
return new StringBuilder("String value: '")
.append(s)
.append("' is not valid with respect to pattern: [\\\\r\\\\n\\\\t\\\\S]+")
.toString();
}

/**
* Helper method to check if there is unsupported unicode.
* @param s the source of the character
* @param ch the character to check
* @throws IllegalStateException indicating an invalid unicode character was detected.
*
* @implNote Per the specification: Strings SHOULD not contain Unicode character points below 32
* except for u0009 (horizontal tab), u0010 (carriage return) and u0013 (line feed).
*/
private static void checkUnsupportedUnicode(String s, char ch) {
if (FHIRModelConfig.shouldCheckUnicodeControlChars() && UNSUPPORTED_UNICODE.contains(ch)) {
throw new IllegalStateException(buildUnicodeException(s));
}
}

/**
* wraps the unicode exception string
* @param s
* @return
*/
private static String buildUnicodeException(String s) {
return new StringBuilder("String value contains unsupported unicode values: [\\0000-0008,0011,0012,0014-0031] value=[")
.append(s)
.append(']')
.toString();
}

/**
* A string which has at least one character and no leading or trailing whitespace and where there is no whitespace other
* than single spaces in the contents.
Expand Down Expand Up @@ -147,6 +203,7 @@ public static void checkCode(String s) {
}
previousIsSpace = true;
} else {
checkUnsupportedUnicode(s, current);
if (previousIsSpace) {
previousIsSpace = false;
}
Expand Down Expand Up @@ -175,6 +232,7 @@ public static void checkId(String s) {
}
for (int i = 0; i < s.length(); i++) {
char c = s.charAt(i);
// @implNote By implication, this excludes invalid unicode.
//45 = '-'
//46 = '.'
//48 = '0'
Expand Down Expand Up @@ -205,7 +263,9 @@ public static void checkUri(String s) {
throw new IllegalStateException(String.format("Uri value length: %d is greater than maximum allowed length: %d", s.length(), MAX_STRING_LENGTH));
}
for (int i = 0; i < s.length(); i++) {
if (Character.isWhitespace(s.charAt(i))) {
char ch = s.charAt(i);
checkUnsupportedUnicode(s, ch);
if (Character.isWhitespace(ch)) {
throw new IllegalStateException(String.format("Uri value: '%s' must not contain whitespace", s));
}
}
Expand Down Expand Up @@ -800,4 +860,4 @@ private static boolean hasDataAbsentReasonExtension(Element element) {
return false;
}

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* (C) Copyright IBM Corp. 2021
*
* SPDX-License-Identifier: Apache-2.0
*/

package com.ibm.fhir.model.util.test.unicode;

import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.ReadableByteChannel;

import com.ibm.fhir.model.util.test.unicode.strategy.CharacterControlStrategy;

/**
* InjectCharacterChannel modifies the Channel based on the CharacterControlStategy.
*
* @implNote The injection (today) occurs on the first key. It's designed to be extensible, such that
* other hooks on the stream are used for injection.
*/
public class InjectCharacterChannel implements ReadableByteChannel {

private ReadableByteChannel channel;
private CharacterControlStrategy strategy;
private boolean first = true;

public InjectCharacterChannel(ReadableByteChannel channel, CharacterControlStrategy strategy) {
this.channel = channel;
this.strategy = strategy;
}

@Override
public void close() throws IOException {
channel.close();
}

@Override
public boolean isOpen() {
return channel.isOpen();
}

@Override
public int read(ByteBuffer dst) throws IOException {
/*
* Based on the strategy, the ReadableByteChannel is updated.
*/
ByteBuffer temp = ByteBuffer.allocate(4096);
int len = channel.read(temp);

if (strategy.isApplicable(len, temp) && len > 0) {
byte[] pre = strategy.pre();

// Translate the body to a smaller byte array.
if (first) {
len++;
}
byte[] body = new byte[len];

int pad = 0;
for (int index = 0; index < len; index++) {
byte t = temp.get(index - pad);
int x = t;
if (x == 34 && first) {
body[index] = t;
index++;
body[index] = strategy.pre()[0];
pad = 1;
first = false;
} else {
body[index] = t;
}
}
byte[] post = strategy.post();

// Don't reallocate the dst buffer above.
dst.put(body);
//len += post.length;
} else if (len > 0) {
byte[] body = new byte[len];
for (int index = 0; index < len; index++) {
byte t = temp.get(index);
body[index] = t;
}
dst = dst.put(body);
}
return len;
}
}
Loading

0 comments on commit 37a0eb2

Please sign in to comment.