From 40b62791ee5479e167859054a92b24b8563429d2 Mon Sep 17 00:00:00 2001
From: Alexander Kiel <alexanderkiel@gmx.net>
Date: Fri, 10 Dec 2021 11:04:28 +0100
Subject: [PATCH] Ensure Error Response on Invalid Value in FHIR Search
In the default lenient handling, invalid search values were ignored.
This is not the intended behaviour. Only unknown search parameters
should be ignored, but not invalid values.
---
modules/db/src/blaze/db/api_spec.clj | 2 +-
modules/db/src/blaze/db/node.clj | 9 +--
modules/db/test/blaze/db/api_test.clj | 32 +++++++--
.../src/blaze/interaction/search_type.clj | 4 +-
.../blaze/interaction/search_type_test.clj | 34 ++++++++++
.../blaze/interaction/transaction_test.clj | 67 ++++++++++++++++++-
6 files changed, 135 insertions(+), 13 deletions(-)
diff --git a/modules/db/src/blaze/db/api_spec.clj b/modules/db/src/blaze/db/api_spec.clj
index 86fae5f05..8422a781f 100644
--- a/modules/db/src/blaze/db/api_spec.clj
+++ b/modules/db/src/blaze/db/api_spec.clj
@@ -80,7 +80,7 @@
:args (s/cat :node-or-db (s/or :node :blaze.db/node :db :blaze.db/db)
:type :fhir.type/name
:clauses :blaze.db.query/clauses)
- :ret :blaze.db/query)
+ :ret (s/or :query :blaze.db/query :anomaly ::anom/anomaly))
diff --git a/modules/db/src/blaze/db/node.clj b/modules/db/src/blaze/db/node.clj
index 57e9200d0..c40c062c5 100644
--- a/modules/db/src/blaze/db/node.clj
+++ b/modules/db/src/blaze/db/node.clj
@@ -285,10 +285,11 @@
(batch-db/->TypeQuery (codec/tid type) (seq clauses))))
(-compile-type-query-lenient [_ type clauses]
- (if-let [clauses (seq (resolve-search-params search-param-registry type
- clauses true))]
- (batch-db/->TypeQuery (codec/tid type) clauses)
- (batch-db/->EmptyTypeQuery (codec/tid type))))
+ (when-ok [clauses (resolve-search-params search-param-registry type clauses
+ true)]
+ (if-let [clauses (seq clauses)]
+ (batch-db/->TypeQuery (codec/tid type) clauses)
+ (batch-db/->EmptyTypeQuery (codec/tid type)))))
(-compile-system-query [_ clauses]
(when-ok [clauses (resolve-search-params search-param-registry "Resource"
diff --git a/modules/db/test/blaze/db/api_test.clj b/modules/db/test/blaze/db/api_test.clj
index 50f67a548..8497d908c 100644
--- a/modules/db/test/blaze/db/api_test.clj
+++ b/modules/db/test/blaze/db/api_test.clj
@@ -2888,7 +2888,12 @@
(given (d/compile-type-query node "Patient" [["foo" "bar"]
["active" "true"]])
::anom/category := ::anom/not-found
- ::anom/message := "The search-param with code `foo` and type `Patient` was not found.")))))
+ ::anom/message := "The search-param with code `foo` and type `Patient` was not found."))
+
+ (testing "invalid date"
+ (given (d/compile-type-query node "Patient" [["birthdate" "invalid"]])
+ ::anom/category := ::anom/incorrect
+ ::anom/message := "Invalid date-time value `invalid` in search parameter `birthdate`.")))))
(deftest compile-type-query-lenient-test
@@ -2897,21 +2902,38 @@
@(d/transact node [[:put {:fhir/type :fhir/Patient :id "0" :active true}]])
(testing "the patient can be found"
- (given @(->> (d/compile-type-query-lenient node "Patient" [["active" "true"]])
+ (given @(->> (d/compile-type-query-lenient
+ node "Patient" [["active" "true"]])
(d/execute-query (d/db node))
(d/pull-many node))
[0 :fhir/type] := :fhir/Patient
[0 :id] := "0"))
(testing "an unknown search-param is ignored"
- (let [clauses [["foo" "bar"] ["active" "true"]]
- query (d/compile-type-query-lenient node "Patient" clauses)]
+ (let [query (d/compile-type-query-lenient
+ node "Patient" [["foo" "bar"] ["active" "true"]])]
(given @(d/pull-many node (d/execute-query (d/db node) query))
[0 :fhir/type] := :fhir/Patient
[0 :id] := "0")
(testing "the clause [\"foo\" \"bar\"] was not used"
- (is (= [["active" "true"]] (d/query-clauses query)))))))))
+ (is (= [["active" "true"]] (d/query-clauses query)))))
+
+ (testing "one unknown search parameter will result in an empty query"
+ (let [query (d/compile-type-query-lenient
+ node "Patient" [["foo" "bar"]])]
+ (given @(d/pull-many node (d/execute-query (d/db node) query))
+ [0 :fhir/type] := :fhir/Patient
+ [0 :id] := "0")
+
+ (testing "the clause [\"foo\" \"bar\"] was not used"
+ (is (empty? (d/query-clauses query)))))))
+
+ (testing "invalid date"
+ (given (d/compile-type-query-lenient
+ node "Patient" [["birthdate" "invalid"]])
+ ::anom/category := ::anom/incorrect
+ ::anom/message := "Invalid date-time value `invalid` in search parameter `birthdate`.")))))
diff --git a/modules/interaction/src/blaze/interaction/search_type.clj b/modules/interaction/src/blaze/interaction/search_type.clj
index a388211d1..fe8072e62 100644
--- a/modules/interaction/src/blaze/interaction/search_type.clj
+++ b/modules/interaction/src/blaze/interaction/search_type.clj
@@ -57,7 +57,7 @@
:clauses clauses})
:else
- (let [query (d/compile-type-query-lenient db type clauses)]
+ (when-ok [query (d/compile-type-query-lenient db type clauses)]
{:handles (execute-query db query page-id)
:clauses (d/query-clauses query)})))
@@ -193,7 +193,7 @@
:clauses clauses})
:else
- (let [query (d/compile-type-query-lenient db type clauses)]
+ (when-ok [query (d/compile-type-query-lenient db type clauses)]
{:total (count (d/execute-query db query))
:clauses (d/query-clauses query)})))
diff --git a/modules/interaction/test/blaze/interaction/search_type_test.clj b/modules/interaction/test/blaze/interaction/search_type_test.clj
index 53c9beab7..306a26348 100644
--- a/modules/interaction/test/blaze/interaction/search_type_test.clj
+++ b/modules/interaction/test/blaze/interaction/search_type_test.clj
@@ -456,6 +456,40 @@
(is (= #fhir/uri"base-url-113047/Patient?active=true&_summary=count&_count=50&__t=1"
(link-url body "self")))))))))))
+ (testing "on invalid date-time"
+ (testing "returns error"
+ (with-handler [handler]
+ []
+ (testing "normal result"
+ (let [{:keys [status body]}
+ @(handler
+ {::reitit/match observation-match
+ ;; the date is already URl decoded and so contains a space instead of a plus
+ :params {"date" "2021-12-09T00:00:00 01:00"}})]
+
+ (is (= 400 status))
+
+ (given body
+ :fhir/type := :fhir/OperationOutcome
+ [:issue 0 :severity] := #fhir/code"error"
+ [:issue 0 :code] := #fhir/code"invalid"
+ [:issue 0 :diagnostics] := "Invalid date-time value `2021-12-09T00:00:00 01:00` in search parameter `date`.")))
+
+ (testing "summary result"
+ (let [{:keys [status body]}
+ @(handler
+ {::reitit/match observation-match
+ ;; the date is already URl decoded and so contains a space instead of a plus
+ :params {"date" "2021-12-09T00:00:00 01:00" "_summary" "count"}})]
+
+ (is (= 400 status))
+
+ (given body
+ :fhir/type := :fhir/OperationOutcome
+ [:issue 0 :severity] := #fhir/code"error"
+ [:issue 0 :code] := #fhir/code"invalid"
+ [:issue 0 :diagnostics] := "Invalid date-time value `2021-12-09T00:00:00 01:00` in search parameter `date`."))))))
+
(testing "on invalid token"
(testing "returns error"
(with-handler [handler]
diff --git a/modules/interaction/test/blaze/interaction/transaction_test.clj b/modules/interaction/test/blaze/interaction/transaction_test.clj
index d3a4824c4..12a73ee21 100644
--- a/modules/interaction/test/blaze/interaction/transaction_test.clj
+++ b/modules/interaction/test/blaze/interaction/transaction_test.clj
@@ -63,6 +63,8 @@
[["/Observation"
{:name :Observation/type
:fhir.resource/type "Observation"
+ :get {:middleware [[wrap-db node]]
+ :handler (wrap-params search-type-handler)}
:post create-handler}]
["/Patient"
{:name :Patient/type
@@ -1774,4 +1776,67 @@
(testing "entry response"
(given response
- :status := "200"))))))))
+ :status := "200")))))
+
+ (testing "with date-time search param value"
+ (with-handler [handler]
+ [[[:create
+ {:fhir/type :fhir/Observation :id "0"
+ :effective #fhir/dateTime"2021-12-08T00:00:00+01:00"}]
+ [:create
+ {:fhir/type :fhir/Observation :id "1"
+ :effective #fhir/dateTime"2021-12-09T00:00:00+01:00"}]]]
+
+ (let [{:keys [status] {[{:keys [resource response]}] :entry} :body}
+ @(handler
+ {:body
+ {:fhir/type :fhir/Bundle
+ :type #fhir/code"batch"
+ :entry
+ [{:fhir/type :fhir.Bundle/entry
+ :request
+ {:fhir/type :fhir.Bundle.entry/request
+ :method #fhir/code"GET"
+ :url #fhir/uri"Observation?date=lt2021-12-08T10:00:00%2B01:00"}}]}})]
+
+ (testing "response status"
+ (is (= 200 status)))
+
+ (testing "entry resource"
+ (given resource
+ :fhir/type := :fhir/Bundle
+ :type := #fhir/code"searchset"
+ :total := #fhir/unsignedInt 1
+ [:entry count] := 1))
+
+ (testing "entry response"
+ (given response
+ :status := "200")))
+
+ (testing "without encoding of the plus in the timezone"
+ (let [{:keys [status] {[{:keys [response]}] :entry} :body}
+ @(handler
+ {:body
+ {:fhir/type :fhir/Bundle
+ :type #fhir/code"batch"
+ :entry
+ [{:fhir/type :fhir.Bundle/entry
+ :request
+ {:fhir/type :fhir.Bundle.entry/request
+ :method #fhir/code"GET"
+ :url #fhir/uri"Observation?date=lt2021-12-09T00:00:00+01:00"}}]}})]
+
+ (testing "response status"
+ (is (= 200 status)))
+
+ (testing "returns error"
+ (testing "with status"
+ (is (= "400" (:status response))))
+
+ (testing "with outcome"
+ (given (:outcome response)
+ :fhir/type := :fhir/OperationOutcome
+ [:issue 0 :severity] := #fhir/code"error"
+ [:issue 0 :code] := #fhir/code"invalid"
+ [:issue 0 :diagnostics] := "Invalid date-time value `2021-12-09T00:00:00 01:00` in search parameter `date`."
+ [:issue 0 :expression 0] := "Bundle.entry[0]"))))))))))