From 50acfbede3e7f1a87675746b9181e35db9348800 Mon Sep 17 00:00:00 2001 From: Michael Buckley Date: Thu, 4 Feb 2021 10:34:03 -0500 Subject: [PATCH] Reject invalid param prefix. An unrecognized param prefix had been silently ignored. Reject it with a DataFormatException instead. --- .../fhir/rest/param/BaseParamWithPrefix.java | 15 +++-- .../ca/uhn/fhir/rest/param/DateParamTest.java | 61 +++++++++++++++++++ .../r4/FhirResourceDaoR4SearchNoFtTest.java | 22 ------- 3 files changed, 68 insertions(+), 30 deletions(-) create mode 100644 hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/param/DateParamTest.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/BaseParamWithPrefix.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/BaseParamWithPrefix.java index e41faf5af8d..8215996a5f1 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/BaseParamWithPrefix.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/BaseParamWithPrefix.java @@ -20,6 +20,8 @@ package ca.uhn.fhir.rest.param; * #L% */ +import ca.uhn.fhir.parser.DataFormatException; + import static org.apache.commons.lang3.StringUtils.isBlank; public abstract class BaseParamWithPrefix extends BaseParam { @@ -58,8 +60,9 @@ public abstract class BaseParamWithPrefix extends BaseParam if (!isBlank(prefix)) { myPrefix = ParamPrefixEnum.forValue(prefix); - + if (myPrefix == null) { + // prefix doesn't match standard values. Try legacy values switch (prefix) { case ">=": myPrefix = ParamPrefixEnum.GREATERTHAN_OR_EQUALS; @@ -77,14 +80,9 @@ public abstract class BaseParamWithPrefix extends BaseParam myPrefix = ParamPrefixEnum.APPROXIMATE; break; default : - ourLog.warn("Invalid prefix being ignored: {}", prefix); - break; + throw new DataFormatException("Invalid prefix: \"" + prefix + "\""); } - - if (myPrefix != null) { - ourLog.warn("Date parameter has legacy prefix '{}' which has been removed from FHIR. This should be replaced with '{}'", prefix, myPrefix); - } - + ourLog.warn("Date parameter has legacy prefix '{}' which has been removed from FHIR. This should be replaced with '{}'", prefix, myPrefix.getValue()); } } @@ -107,4 +105,5 @@ public abstract class BaseParamWithPrefix extends BaseParam myPrefix = thePrefix; return (T) this; } + } diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/param/DateParamTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/param/DateParamTest.java new file mode 100644 index 00000000000..144bbbb4205 --- /dev/null +++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/param/DateParamTest.java @@ -0,0 +1,61 @@ +package ca.uhn.fhir.rest.param; + +import ca.uhn.fhir.parser.DataFormatException; +import org.junit.jupiter.api.Test; + +import java.time.Month; +import java.time.ZoneId; +import java.time.ZonedDateTime; + +import static org.junit.jupiter.api.Assertions.*; + +public class DateParamTest { + + @Test + public void testBasicParse() { + DateParam input = new DateParam("2020-01-01"); + + // too bad value is a j.u.Date instead of a new JSR-310 type + // DataParam parses using default tz, so go backwards. + ZonedDateTime zonedDateTime = input.getValue().toInstant().atZone(ZoneId.systemDefault()); + assertEquals(2020,zonedDateTime.getYear()); + assertEquals(Month.JANUARY,zonedDateTime.getMonth()); + assertEquals(1,zonedDateTime.getDayOfMonth()); + assertNull(input.getPrefix()); + } + + @Test + public void testBadDateFormat() { + try { + DateParam input = new DateParam("09-30-1960"); + fail(); + } catch (DataFormatException e) { + // expected + } + } + + @Test + public void testPrefixParse() { + DateParam input = new DateParam("gt2020-01-01"); + + assertEquals(ParamPrefixEnum.GREATERTHAN, input.getPrefix()); + } + + @Test + public void testLegacyPrefixParse() { + DateParam input = new DateParam(">2020-01-01"); + + assertEquals(ParamPrefixEnum.GREATERTHAN, input.getPrefix()); + } + + @Test + public void testJunkDateIssue1465() { + // Issue 1464 - the string "junk" wasn't returning 400 as expected. Parsed as a prefix instead, and discarded. + try { + DateParam input = new DateParam("junk"); + fail(); + } catch (DataFormatException e) { + // expected + } + } +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index 2b820b2a280..d63d42a2deb 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java @@ -5163,28 +5163,6 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { } } - @Test - public void testIssueDateFormatError1465() { - try { - SearchParameterMap map = SearchParameterMap.newSynchronous(); - map.add("birthdate", new DateParam("09-30-1960")); - myPatientDao.search(map); - fail(); - } catch (DataFormatException e) { - assertEquals("Invalid date/time format: \"09-30-1960\"", e.getMessage()); - } - - try { - SearchParameterMap map = SearchParameterMap.newSynchronous(); - map.add("birthdate", new DateParam("junk")); - myPatientDao.search(map); - fail(); - } catch (DataFormatException e) { - assertEquals("Invalid date/time format: \"junk\"", e.getMessage()); - } - } - - @Test public void testDateSearchParametersShouldBeTimezoneIndependent() {