PLUGINS-364 validate DA/TM query parameters; stop accepting wildcards

CHANGELOG.md

@@ -2,6 +2,41 @@
 
 All notable changes to the XNAT DICOMweb Plugin will be documented in this file.
 
+## [Unreleased]
+
+### Fixed
+- **QIDO-RS date and time parameters are now validated** — a malformed
+  `StudyDate` or `StudyTime` returns **HTTP 400** with an
+  `InvalidParameter` error, per PS3.18 §10.6.3.1. Previously such
+  values produced an HTTP 200 with an empty result list, because the
+  `BadRequestException` raised while building the query was caught by
+  the catch-all in `XnatDicomServiceImpl.searchStudies` and logged
+  rather than propagated.
+  - A malformed but 8-digit date such as `StudyDate=20251345` reached
+    the database as `2025-13-45` and failed there, with the same
+    swallowed result.
+  - A malformed date of any other length, such as `StudyDate=garbage`,
+    matched no filter branch at all and was silently dropped, so the
+    query returned **every** study rather than none.
+  - `StudyTime` values were never validated; an invalid time simply
+    matched nothing.
+- **Partial-precision `StudyTime` values are now accepted.** PS3.5 §6.2
+  allows the TM components `MM`, `SS` and `FFFFFF` to be unspecified
+  from the right, so `10`, `1030` and `103000.5` are valid times. The
+  range parser previously required exactly 6 digits, which rejected
+  `StudyTime=1000-1800` — the worked example in PS3.4 §C.2.2.2.5.4.
+  Leap seconds (`SS`=60) are also accepted.
+
+### Changed
+- **Wildcards are no longer accepted in `StudyDate` / `StudyTime`.**
+  PS3.4 §C.2.2.2.4 defines Wild Card Matching only for attributes of
+  VR AE, CS, LO, LT, PN, SH, ST, UC, UR and UT; DA and TM are excluded.
+  `StudyDate=2025*` now returns 400. A bare `*` is still accepted as
+  universal matching, per the §C.2.2.2.4 note that "Wild Card Matching
+  on a value of `*` is equivalent to Universal Matching". Wildcard
+  matching on `PatientName`, `SeriesDescription` and other string
+  attributes is unaffected.
+
 ## [1.2.0] - 2026-04-09
 
 ### Added

CONFORMANCE.md

@@ -130,6 +130,26 @@ Clients relying on the PS3.18 convention will not detect that more
 results remain. Clients aware of the plugin's `X-Total-Count` can
 compute the remainder as `X-Total-Count − offset − returned`.
 
+### 0.11 A partial-precision `StudyTime` matches every time sharing its components
+
+PS3.5 §6.2 allows a TM value to omit components from the right,
+"which indicates that the Value is not precise to the precision of
+those unspecified components", but PS3.4 §C.2.2.2.1 defines Single
+Value Matching as matching "exactly the value specified in the
+request" without saying how the two interact.
+
+The plugin resolves this by comparing only the components the client
+supplied: `StudyTime=10` matches every session in the 10:00 hour and
+`StudyTime=1030` every session in the 10:30 minute, rather than
+matching only 10:00:00 and 10:30:00 exactly. A fractional second in
+a single value is accepted but ignored, because XNAT stores no
+sub-second precision.
+
+Note that this differs from the *range* case, where an unspecified
+component resolves to zero because a range endpoint denotes a single
+instant — `StudyTime=1000-1800` ends at 18:00:00 exactly, per the
+worked example in PS3.4 §C.2.2.2.5.4.
+
 ---
 
 ## 1. Overview
@@ -376,8 +396,8 @@ enforce TLS itself and does not configure CORS (see Section 10.2).
 | Parameter            | Levels                  | DICOM Tag    | Notes                                  |
 |----------------------|-------------------------|--------------|----------------------------------------|
 | `StudyInstanceUID`   | Study, Series, Instance | (0020,000D)  | UID-list (comma-separated) not supported |
-| `StudyDate`          | Study                   | (0008,0020)  | Exact, wildcard, or range              |
-| `StudyTime`          | Study                   | (0008,0030)  | Exact, wildcard, or range              |
+| `StudyDate`          | Study                   | (0008,0020)  | Exact or range; malformed values → 400 |
+| `StudyTime`          | Study                   | (0008,0030)  | Exact or range; malformed values → 400 |
 | `PatientName`        | Study                   | (0010,0010)  |                                        |
 | `PatientID`          | Study                   | (0010,0020)  |                                        |
 | `AccessionNumber`    | Study                   | (0008,0050)  | Matches the XNAT session ID (see 6.5)  |
@@ -402,16 +422,35 @@ enforce TLS itself and does not configure CORS (see Section 10.2).
   files.
 - **Wildcard** — `*` and `?`, translated to SQL `ILIKE` patterns for
   study-level keys and to regex-like Java matching for lower levels.
-  No escape mechanism for literal `*` / `?`.
+  No escape mechanism for literal `*` / `?`. Per PS3.4 §C.2.2.2.4
+  this applies only to attributes of VR AE, CS, LO, LT, PN, SH, ST,
+  UC, UR and UT, so it is **not** available on `StudyDate` or
+  `StudyTime` — see below.
 - **Range** — supported at all levels, per PS3.4 §C.2.2.2.5.1 (DA)
-  and §C.2.2.2.5.2 (TM). Range endpoints must be full 8-digit
-  `yyyyMMdd` (DA) or 6-digit `HHmmss` (TM); wildcards inside a range
-  endpoint are not permitted. Malformed range values at the study
-  level return HTTP 400.
+  and §C.2.2.2.5.2 (TM). Wildcards inside a range endpoint are not
+  permitted. Endpoints follow the VR grammar of PS3.5 §6.2: a date
+  is a full 8-digit `yyyyMMdd`, while a time may use the
+  partial-precision forms `HH`, `HHmm` and `HHmmss` and an optional
+  1-to-6-digit fractional second. Unspecified time components
+  resolve to zero, so `StudyTime=1000-1800` spans 10:00:00 to
+  18:00:00 — the worked example in PS3.4 §C.2.2.2.5.4.
 - **Universal** — empty parameter value matches everything.
   The DICOM range marker `-` (both bounds omitted) is treated the
-  same way at the study level.
-- **Date format** — DICOM `yyyyMMdd` only.
+  same way at the study level, as is a bare `*` in `StudyDate` or
+  `StudyTime` (PS3.4 §C.2.2.2.4 note: "Wild Card Matching on a value
+  of `*` is equivalent to Universal Matching").
+- **Date and time validation** — `StudyDate` and `StudyTime` values
+  are validated against the DA and TM grammars at the REST boundary.
+  A malformed value returns HTTP 400 with an `InvalidParameter`
+  error naming the parameter, per PS3.18 §10.6.3.1 ("400 (Bad
+  Request) — The was a problem with the request. For example, the
+  Query Parameter syntax is incorrect."). This covers malformed
+  single values (`20251345`), malformed range endpoints
+  (`20250101-nonsense`), malformed range structure
+  (`20250101-20250201-20250301`), and wildcards.
+- **Date format** — DICOM `yyyyMMdd` only. The ISO `yyyy-MM-dd` form
+  is rejected: `-` is the DICOM range separator, so `2025-01-15`
+  parses as a three-part range and returns 400.
 
 ### 6.4 Pagination
 

docs/QIDO_RS_IMPLEMENTATION.md

@@ -225,8 +225,38 @@ StudyTime=080000-170000
 
 # Combined date and time
 StudyDate=20240101-20240131&StudyTime=080000-170000
+
+# Open-ended ranges
+StudyDate=20240101-        # on or after Jan 1
+StudyDate=-20240131        # on or before Jan 31
+
+# Times may use partial precision (PS3.5 §6.2)
+StudyTime=10               # the 10:00 hour
+StudyTime=1000-1800        # 10:00:00 through 18:00:00
 ```
 
+### Validation
+
+`StudyDate` and `StudyTime` values are validated against the DICOM DA
+and TM grammars before the query runs. A malformed value returns
+**HTTP 400** with an `InvalidParameter` error naming the parameter —
+it does *not* return an empty result set.
+
+```bash
+StudyDate=20251345                    # 400 — no such month/day
+StudyDate=2025-01-15                  # 400 — "-" is the range separator,
+                                      #       so this parses as a 3-part range
+StudyDate=20250101-nonsense           # 400 — malformed range endpoint
+StudyTime=250000                      # 400 — hour out of range
+StudyDate=2025*                       # 400 — wildcards are not defined for
+                                      #       dates/times (PS3.4 §C.2.2.2.4)
+```
+
+Wildcard matching applies only to string-valued attributes such as
+`PatientName` and `SeriesDescription`. A bare `StudyDate=*` is
+accepted as universal matching and is equivalent to omitting the
+parameter.
+
 ---
 
 ## Pagination

docs/USER_GUIDE.md

@@ -310,6 +310,19 @@ Add query parameters to filter results:
 ```bash
 ?StudyDate=20241201                    # Specific date
 ?StudyDate=20240101-20241231          # Date range (all of 2024)
+?StudyDate=20240101-                  # On or after Jan 1
+?StudyDate=-20241231                  # On or before Dec 31
+```
+
+Dates use the DICOM `YYYYMMDD` format. A malformed date returns
+**400 Bad Request** rather than an empty result list:
+
+```bash
+?StudyDate=2024-12-01                 # 400 — use 20241201; a hyphen
+                                      #       means a range, not a separator
+?StudyDate=20241345                   # 400 — no month 13
+?StudyDate=2024*                      # 400 — wildcards work on names,
+                                      #       not on dates or times
 ```
 
 **By Patient ID:**
@@ -384,8 +397,8 @@ curl -u user:pass \
 |-----------|------------|---------|------------------|
 | `PatientName` | Studies | `DOE^JOHN` | Yes (`*`, `?`) |
 | `PatientID` | Studies | `P12345` | No |
-| `StudyDate` | Studies | `20241201-20241231` | Range supported |
-| `StudyTime` | Studies | `080000-170000` | Range supported |
+| `StudyDate` | Studies | `20241201-20241231` | No — range supported; malformed values return 400 |
+| `StudyTime` | Studies | `080000-170000` | No — range supported; malformed values return 400 |
 | `StudyInstanceUID` | Studies | `1.2.840...` | No |
 | `AccessionNumber` | Studies | `ACC123` | No |
 | `Modality` | Studies, Series | `CT` | No |

src/main/java/org/nrg/xnat/dicomweb/service/impl/XnatDicomServiceImpl.java

@@ -74,6 +74,7 @@
 import org.nrg.xnat.dicomweb.service.SiteWideProjectFilter;
 import org.nrg.xnat.dicomweb.service.XnatDicomService;
 import org.nrg.xnat.dicomweb.util.BulkDataHandler;
+import org.nrg.xnat.dicomweb.util.DicomDateTimeValues;
 import org.nrg.xnat.dicomweb.util.DicomRangeParser;
 import org.nrg.xnat.dicomweb.util.DicomWebUtils;
 import org.nrg.xnat.utils.CatalogUtils;
@@ -468,16 +469,13 @@ private static void appendStudyDateClauses(
                 sql.append("AND e.date <= CAST(:q_study_date_end AS DATE) ");
                 params.addValue("q_study_date_end", r.end.toString());
             }
-        } else if (studyDate.contains("*") || studyDate.contains("?")) {
-            appendIlikeFilter(sql, params, studyDate,
-                    "TO_CHAR(e.date, 'YYYYMMDD')", "q_study_date");
-        } else if (studyDate.length() == 8) {
-            String sqlDate = studyDate.substring(0, 4) + "-"
-                    + studyDate.substring(4, 6) + "-"
-                    + studyDate.substring(6, 8);
-            sql.append("AND e.date = CAST(:q_study_date AS DATE) ");
-            params.addValue("q_study_date", sqlDate);
+            return;
         }
+        // Single Value Matching, PS3.4 §C.2.2.2.1. The value reached
+        // here through QidoQueryParamParser, so it is a well-formed DA.
+        sql.append("AND e.date = CAST(:q_study_date AS DATE) ");
+        params.addValue("q_study_date",
+                DicomDateTimeValues.parseDate("StudyDate", studyDate).toString());
     }
 
     private static void appendStudyTimeClauses(
@@ -496,10 +494,18 @@ private static void appendStudyTimeClauses(
                 sql.append("AND e.time <= CAST(:q_study_time_end AS TIME) ");
                 params.addValue("q_study_time_end", r.end.toString());
             }
-        } else {
-            appendIlikeFilter(sql, params, studyTime,
-                    "TO_CHAR(e.time, 'HH24MISS')", "q_study_time");
+            return;
         }
+        // Single Value Matching, PS3.4 §C.2.2.2.1. TM permits partial
+        // precision (PS3.5 §6.2), and a value "not precise to the
+        // precision of those unspecified components" matches every
+        // stored time sharing the specified components — so compare
+        // only as many leading digits as the client supplied.
+        String prefix = DicomDateTimeValues.timeMatchPrefix(studyTime);
+        sql.append("AND LEFT(TO_CHAR(e.time, 'HH24MISS'), :q_study_time_digits) "
+                + "= :q_study_time ");
+        params.addValue("q_study_time_digits", prefix.length());
+        params.addValue("q_study_time", prefix);
     }
 
     @Override
@@ -643,6 +649,12 @@ public List<Attributes> searchStudies(UserI user, String projectId, Attributes q
             }
 
             log.debug("Found {} studies{}", results.size(), siteWide ? " (site-wide)" : " in project " + projectId);
+        } catch (DicomWebException e) {
+            // A client error (e.g. a malformed query value) must reach
+            // GlobalExceptionHandler and become its own status code.
+            // Folding it into the catch-all below would report an
+            // invalid request as an empty but successful result set.
+            throw e;
         } catch (Exception e) {
             log.error("Error searching studies{}", projectId != null ? " in project " + projectId : " (site-wide)", e);
         }