Improve performance of base64 validation

This commit is contained in:
James Agnew 2018-01-30 17:37:12 -06:00
parent 1c5a07b5a0
commit b61887e841
10 changed files with 168 additions and 63 deletions

View File

@ -20,25 +20,32 @@ package ca.uhn.fhir.rest.param;
* #L% * #L%
*/ */
import java.util.ArrayList;
import java.util.List;
import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.model.api.IQueryParameterAnd; import ca.uhn.fhir.model.api.IQueryParameterAnd;
import ca.uhn.fhir.model.api.IQueryParameterOr; import ca.uhn.fhir.model.api.IQueryParameterOr;
import ca.uhn.fhir.rest.api.QualifiedParamList; import ca.uhn.fhir.rest.api.QualifiedParamList;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import java.util.ArrayList;
import java.util.List;
public abstract class BaseAndListParam<T extends IQueryParameterOr<?>> implements IQueryParameterAnd<T> { public abstract class BaseAndListParam<T extends IQueryParameterOr<?>> implements IQueryParameterAnd<T> {
private List<T> myValues=new ArrayList<T>(); private List<T> myValues = new ArrayList<>();
public abstract BaseAndListParam<T> addAnd(T theValue);
public BaseAndListParam<T> addValue(T theValue) { public BaseAndListParam<T> addValue(T theValue) {
myValues.add(theValue); myValues.add(theValue);
return this; return this;
} }
public abstract BaseAndListParam<T> addAnd(T theValue); @Override
public List<T> getValuesAsQueryTokens() {
return myValues;
}
abstract T newInstance();
@Override @Override
public void setValuesAsQueryTokens(FhirContext theContext, String theParamName, List<QualifiedParamList> theParameters) throws InvalidRequestException { public void setValuesAsQueryTokens(FhirContext theContext, String theParamName, List<QualifiedParamList> theParameters) throws InvalidRequestException {
@ -50,11 +57,9 @@ public abstract class BaseAndListParam<T extends IQueryParameterOr<?>> implement
} }
} }
abstract T newInstance();
@Override @Override
public List<T> getValuesAsQueryTokens() { public String toString() {
return myValues; return myValues.toString();
} }

View File

@ -20,22 +20,34 @@ package ca.uhn.fhir.rest.param;
* #L% * #L%
*/ */
import java.util.ArrayList;
import java.util.List;
import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.model.api.IQueryParameterOr; import ca.uhn.fhir.model.api.IQueryParameterOr;
import ca.uhn.fhir.model.api.IQueryParameterType; import ca.uhn.fhir.model.api.IQueryParameterType;
import ca.uhn.fhir.rest.api.QualifiedParamList; import ca.uhn.fhir.rest.api.QualifiedParamList;
import java.util.ArrayList;
import java.util.List;
abstract class BaseOrListParam<MT extends BaseOrListParam<?, ?>, PT extends IQueryParameterType> implements IQueryParameterOr<PT> { abstract class BaseOrListParam<MT extends BaseOrListParam<?, ?>, PT extends IQueryParameterType> implements IQueryParameterOr<PT> {
private List<PT> myList=new ArrayList<PT>(); private List<PT> myList = new ArrayList<>();
// public void addToken(T theParam) { @SuppressWarnings("unchecked")
// Validate.notNull(theParam,"Param can not be null"); public MT add(PT theParameter) {
// myList.add(theParam); if (theParameter != null) {
// } myList.add(theParameter);
}
return (MT) this;
}
public abstract MT addOr(PT theParameter);
@Override
public List<PT> getValuesAsQueryTokens() {
return myList;
}
abstract PT newInstance();
@Override @Override
public void setValuesAsQueryTokens(FhirContext theContext, String theParamName, QualifiedParamList theParameters) { public void setValuesAsQueryTokens(FhirContext theContext, String theParamName, QualifiedParamList theParameters) {
@ -47,21 +59,9 @@ abstract class BaseOrListParam<MT extends BaseOrListParam<?, ?>, PT extends IQue
} }
} }
abstract PT newInstance();
public abstract MT addOr(PT theParameter);
@SuppressWarnings("unchecked")
public MT add(PT theParameter) {
if (theParameter != null) {
myList.add(theParameter);
}
return (MT) this;
}
@Override @Override
public List<PT> getValuesAsQueryTokens() { public String toString() {
return myList; return myList.toString();
} }
} }

View File

@ -279,21 +279,23 @@ public abstract class RequestDetails {
} }
public Map<String, List<String>> getUnqualifiedToQualifiedNames() { public Map<String, List<String>> getUnqualifiedToQualifiedNames() {
for (String next : myParameters.keySet()) { if (myUnqualifiedToQualifiedNames == null) {
for (int i = 0; i < next.length(); i++) { for (String next : myParameters.keySet()) {
char nextChar = next.charAt(i); for (int i = 0; i < next.length(); i++) {
if (nextChar == ':' || nextChar == '.') { char nextChar = next.charAt(i);
if (myUnqualifiedToQualifiedNames == null) { if (nextChar == ':' || nextChar == '.') {
myUnqualifiedToQualifiedNames = new HashMap<>(); if (myUnqualifiedToQualifiedNames == null) {
myUnqualifiedToQualifiedNames = new HashMap<>();
}
String unqualified = next.substring(0, i);
List<String> list = myUnqualifiedToQualifiedNames.get(unqualified);
if (list == null) {
list = new ArrayList<>(4);
myUnqualifiedToQualifiedNames.put(unqualified, list);
}
list.add(next);
break;
} }
String unqualified = next.substring(0, i);
List<String> list = myUnqualifiedToQualifiedNames.get(unqualified);
if (list == null) {
list = new ArrayList<>(4);
myUnqualifiedToQualifiedNames.put(unqualified, list);
}
list.add(next);
break;
} }
} }
} }

View File

@ -96,7 +96,7 @@ public abstract class BaseQueryParameter implements IParameter {
@Override @Override
public Object translateQueryParametersIntoServerArgument(RequestDetails theRequest, BaseMethodBinding<?> theMethodBinding) throws InternalErrorException, InvalidRequestException { public Object translateQueryParametersIntoServerArgument(RequestDetails theRequest, BaseMethodBinding<?> theMethodBinding) throws InternalErrorException, InvalidRequestException {
List<QualifiedParamList> paramList = new ArrayList<QualifiedParamList>(); List<QualifiedParamList> paramList = new ArrayList<>();
String name = getName(); String name = getName();
parseParams(theRequest, paramList, name, null); parseParams(theRequest, paramList, name, null);

View File

@ -165,7 +165,7 @@ public class SearchDefaultMethodDstu3Test {
assertThat(ourLastParam1.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens(), hasSize(1)); assertThat(ourLastParam1.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens(), hasSize(1));
assertEquals("val1", ourLastParam1.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().get(0).getValue()); assertEquals("val1", ourLastParam1.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().get(0).getValue());
assertThat(ourLastParam2.getValuesAsQueryTokens(), hasSize(2)); assertThat(ourLastParam2.toString(), ourLastParam2.getValuesAsQueryTokens(), hasSize(2));
assertThat(ourLastParam2.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens(), hasSize(1)); assertThat(ourLastParam2.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens(), hasSize(1));
assertEquals("val2", ourLastParam2.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().get(0).getValue()); assertEquals("val2", ourLastParam2.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().get(0).getValue());
assertEquals("val2e", ourLastParam2.getValuesAsQueryTokens().get(1).getValuesAsQueryTokens().get(0).getValue()); assertEquals("val2e", ourLastParam2.getValuesAsQueryTokens().get(1).getValuesAsQueryTokens().get(0).getValue());

View File

@ -182,11 +182,21 @@
<scope>test</scope> <scope>test</scope>
</dependency> </dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-context</artifactId>
<scope>test</scope>
</dependency>
<dependency> <dependency>
<groupId>org.springframework</groupId> <groupId>org.springframework</groupId>
<artifactId>spring-web</artifactId> <artifactId>spring-web</artifactId>
<scope>test</scope> <scope>test</scope>
</dependency> </dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-context</artifactId>
<scope>test</scope>
</dependency>
</dependencies> </dependencies>
<reporting> <reporting>

View File

@ -15,6 +15,12 @@ import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.rest.client.exceptions.FhirClientConnectionException; import ca.uhn.fhir.rest.client.exceptions.FhirClientConnectionException;
import ca.uhn.fhir.rest.client.exceptions.FhirClientInappropriateForServerException; import ca.uhn.fhir.rest.client.exceptions.FhirClientInappropriateForServerException;
import ca.uhn.fhir.util.TestUtil; import ca.uhn.fhir.util.TestUtil;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider;
import org.springframework.core.type.filter.AssignableTypeFilter;
import org.springframework.core.type.filter.TypeFilter;
import java.util.Set;
public class ExceptionPropertiesTest { public class ExceptionPropertiesTest {
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ExceptionPropertiesTest.class); private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ExceptionPropertiesTest.class);
@ -37,13 +43,15 @@ public class ExceptionPropertiesTest {
@SuppressWarnings("deprecation") @SuppressWarnings("deprecation")
@Test @Test
public void testExceptionsAreGood() throws Exception { public void testExceptionsAreGood() throws Exception {
ImmutableSet<ClassInfo> classes = ClassPath.from(Thread.currentThread().getContextClassLoader()).getTopLevelClasses(BaseServerResponseException.class.getPackage().getName()); ClassPathScanningCandidateComponentProvider scanner = new ClassPathScanningCandidateComponentProvider(false);
assertTrue(classes.size() > 5); scanner.addIncludeFilter(new AssignableTypeFilter(BaseServerResponseException.class));
Set<BeanDefinition> classes = scanner.findCandidateComponents(BaseServerResponseException.class.getPackage().getName());
assertTrue(classes.toString(), classes.size() > 5);
for (ClassInfo classInfo : classes) { for (BeanDefinition classInfo : classes) {
ourLog.info("Scanning {}", classInfo.getName()); ourLog.info("Scanning {}", classInfo.getBeanClassName());
Class<?> next = Class.forName(classInfo.getName()); Class<?> next = Class.forName(classInfo.getBeanClassName());
assertNotNull(next); assertNotNull(next);
if (next == getClass()) { if (next == getClass()) {
@ -69,7 +77,7 @@ public class ExceptionPropertiesTest {
try { try {
next.getConstructor(String.class, IBaseOperationOutcome.class); next.getConstructor(String.class, IBaseOperationOutcome.class);
} catch (NoSuchMethodException e) { } catch (NoSuchMethodException e) {
fail(classInfo.getName() + " has no constructor with params: (String, IBaseOperationOutcome)"); fail(classInfo.getBeanClassName() + " has no constructor with params: (String, IBaseOperationOutcome)");
} }
} }

View File

@ -1340,15 +1340,27 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat
String encoded = e.primitiveValue(); String encoded = e.primitiveValue();
if (isNotBlank(encoded)) { if (isNotBlank(encoded)) {
/* /*
* Note: Regex comes from: https://stackoverflow.com/questions/8571501/how-to-check-whether-the-string-is-base64-encoded-or-not
*
* Technically this is not bulletproof as some invalid base64 won't be caught, * Technically this is not bulletproof as some invalid base64 won't be caught,
* but I think it's good enough. The original code used Java8 Base64 decoder * but I think it's good enough. The original code used Java8 Base64 decoder
* but I've replaced it with a regex for 2 reasons: * but I've replaced it with a regex for 2 reasons:
* 1. This code will run on any version of Java * 1. This code will run on any version of Java
* 2. This code doesn't actually decode, which is much easier on memory use for big payloads * 2. This code doesn't actually decode, which is much easier on memory use for big payloads
*/ */
if (!encoded.matches("^([A-Za-z0-9+/]{4})*([A-Za-z0-9+/]{4}|[A-Za-z0-9+/]{3}=|[A-Za-z0-9+/]{2}==)$")) { int charCount = 0;
for (int i = 0; i < encoded.length(); i++) {
char nextChar = encoded.charAt(i);
if (Character.isWhitespace(nextChar)) {
continue;
}
if (Character.isLetterOrDigit(nextChar)) {
charCount++;
}
if (nextChar == '/' || nextChar == '=' || nextChar == '+') {
charCount++;
}
}
if (charCount > 0 && charCount % 4 != 0) {
String value = encoded.length() < 100 ? encoded : "(snip)"; String value = encoded.length() < 100 ? encoded : "(snip)";
rule(errors, IssueType.INVALID, e.line(), e.col(), path, false, "The value \"{0}\" is not a valid Base64 value", value); rule(errors, IssueType.INVALID, e.line(), e.col(), path, false, "The value \"{0}\" is not a valid Base64 value", value);
} }

View File

@ -17,6 +17,7 @@ import java.io.InputStream;
import java.util.*; import java.util.*;
import java.util.zip.GZIPInputStream; import java.util.zip.GZIPInputStream;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.validation.FhirValidator; import ca.uhn.fhir.validation.FhirValidator;
import ca.uhn.fhir.validation.ResultSeverityEnum; import ca.uhn.fhir.validation.ResultSeverityEnum;
import ca.uhn.fhir.validation.SingleValidationMessage; import ca.uhn.fhir.validation.SingleValidationMessage;
@ -247,6 +248,15 @@ public class FhirInstanceValidatorR4Test {
} }
@Test
public void testLargeBase64() throws IOException {
String input = IOUtils.toString(FhirInstanceValidatorR4Test.class.getResourceAsStream("/r4/diagnosticreport-example-gingival-mass.json"), Constants.CHARSET_UTF8);
ValidationResult output = myVal.validateWithResult(input);
List<SingleValidationMessage> errors = logResultsAndReturnNonInformationalOnes(output);
assertEquals(0, errors.size());
}
@Test @Ignore @Test @Ignore
public void testValidateDocument() throws Exception { public void testValidateDocument() throws Exception {
String vsContents = IOUtils.toString(FhirInstanceValidatorR4Test.class.getResourceAsStream("/sample-document.xml"), "UTF-8"); String vsContents = IOUtils.toString(FhirInstanceValidatorR4Test.class.getResourceAsStream("/sample-document.xml"), "UTF-8");

File diff suppressed because one or more lines are too long