Fix problems in unindexing servlet and documentation (dicoogle#744)

* Clarify and correct documentation around indexing/unindexing operations

- the indexing operation does a recursive walk over a URI,
  but the unindexing operation does not
- clarify how to specify multi-value query parameters

* Fix unindex servlet in series & study unindexing

- fetch the correct parameter list for expansion into queries

* Sanitize DICOM UID input in unindex servlet

- only accept valid DICOM UIDS in respective query parameters

Author: Enet4

dicoogle/src/main/java/pt/ua/dicoogle/server/web/servlets/management/UnindexServlet.java

@@ -26,6 +26,7 @@
 import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
@@ -90,7 +91,13 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S
         long failed = 0;
         long notfound = 0;
 
-        Collection<URI> uris = resolveURIs(paramUri, paramSop, paramSeries, paramStudy);
+        Collection<URI> uris;
+        try {
+            uris = resolveURIs(paramUri, paramSop, paramSeries, paramStudy);
+        } catch (IllegalArgumentException ex) {
+            ResponseUtil.sendError(resp, 400, ex.getMessage());
+            return;
+        }
 
         // if only one entry, do it inline
         if (uris.size() <= 1) {
@@ -145,18 +152,30 @@ private static Collection<URI> resolveURIs(String[] paramUri, String[] paramSop,
             return Stream.of(paramUri).map(URI::create).collect(Collectors.toList());
         }
         String attribute = null;
+        String[] values;
         if (paramSop != null) {
             attribute = "SOPInstanceUID";
+            values = paramSop;
         } else if (paramSeries != null) {
             attribute = "SeriesInstanceUID";
+            values = paramSeries;
         } else if (paramStudy != null) {
             attribute = "StudyInstanceUID";
+            values = paramStudy;
+        } else {
+            throw new IllegalArgumentException("No valid parameters to resolve URIs");
         }
 
         final String dcmAttribute = attribute;
+
+        // validate UIDs
+        for (String uid: values) {
+            sanitizeUID(uid);
+        }
+
         List<String> dicomProviders = ServerSettingsManager.getSettings().getArchiveSettings().getDIMProviders();
         if (dicomProviders.isEmpty()) {
-            return Arrays.stream(paramSop).flatMap(uid -> {
+            return Arrays.stream(values).flatMap(uid -> {
                 // translate to URIs
                 JointQueryTask holder = new JointQueryTask() {
                     @Override
@@ -175,11 +194,23 @@ public void onCompletion() {}
 
         }
         String dicomProvider = dicomProviders.iterator().next();
-        return Arrays.stream(paramSop).flatMap(uid -> {
+        return Arrays.stream(values).flatMap(uid -> {
             // translate to URIs
             QueryInterface dicom = PluginController.getInstance().getQueryProviderByName(dicomProvider, false);
 
             return StreamSupport.stream(dicom.query(dcmAttribute + ":\"" + uid + '"').spliterator(), false);
         }).map(r -> r.getURI()).collect(Collectors.toList());
     }
+
+    private static final Pattern UID_PATTERN = Pattern.compile("[0-9]+(\\.[0-9]+)*");
+
+    private static void sanitizeUID(String uid) {
+        if (uid == null || uid.isEmpty()) {
+            throw new IllegalArgumentException("UID parameter is null or empty");
+        }
+        uid = uid.trim();
+        if (!UID_PATTERN.matcher(uid).matches()) {
+            throw new IllegalArgumentException("Invalid DICOM UID");
+        }
+    }
 }

dicoogle_web_api.yaml

@@ -629,6 +629,8 @@ paths:
           required: true
           schema:
             type: string
+          explode: true
+          style: form
         - in: query
           name: plugin
           description: a list of provider plugins
@@ -660,11 +662,15 @@ paths:
         - in: query
           name: uri
           description:
-            a URI or array of URIs representing the root resource of the files
-            to be unindexed
+            A URI or array of URIs representing the files to be unindexed.
+            Unlike [`addIndexTaskURI`](#/paths/~1management~1tasks~1index/post),
+            there is no recursive walk
+            and the URIs must point directly to the files.
           required: false
           schema:
             type: string
+          explode: true
+          style: form
         - in: query
           name: SOPInstanceUID
           description:
@@ -673,24 +679,32 @@ paths:
           required: false
           schema:
             type: string
+          explode: true
+          style: form
         - in: query
           name: SeriesInstanceUID
           description: a UID or list of UIDs representing the DICOM series to be unindexed
           required: false
           schema:
             type: string
+          explode: true
+          style: form
         - in: query
           name: StudyInstanceUID
           description: a UID or list of UIDs representing the DICOM studies to be unindexed
           required: false
           schema:
             type: string
+          explode: true
+          style: form
         - in: query
           name: provider
           description: a list of provider plugins
           required: false
           schema:
             type: string
+          explode: true
+          style: form
       responses:
         "200":
           description: Successful operation
@@ -701,7 +715,7 @@ paths:
       tags:
         - Index
       summary: Request that the file at the given URI is permanently removed
-      operationId: addUnindexTaskURI
+      operationId: addRemoveTaskURI
       parameters:
         - in: query
           name: uri
@@ -711,6 +725,8 @@ paths:
           required: true
           schema:
             type: string
+          explode: true
+          style: form
       responses:
         "200":
           description: Successful operation