From 6ffe2cd10eaad72d42e9f87e0dc34145a2c06f46 Mon Sep 17 00:00:00 2001
From: Albert Louis Rossi <arossi@fnal.gov>
Date: Thu, 9 Nov 2023 08:27:34 -0600
Subject: [PATCH] dcache-frontend,common:  check parsing of Duration in STAGE

Motivation:

See https://github.com/dCache/dcache/issues/7414
`tape API/bulk: seem to choke on any interval other than "PND" and "PTNH"`

Modification:

Solves the first part.  Whereever `Duration.parse` is used
with a REST request (Bulk, QoS), we can now return a bad request
response with a detailed error message.

Result:

No mysterious silent failure.

Target: master
Request: 9.2
Request: 9.1
Request: 9.0
Request: 8.2
Patch: https://rb.dcache.org/r/14161/
Bug: #7414
Requires-notes: yes
Acked-by: Dmitry
---
 .../qos/DefaultQoSPolicyJsonDeserializer.java      |  3 ++-
 .../src/main/java/org/dcache/util/TimeUtils.java   | 14 ++++++++++++++
 .../restful/resources/tape/StageResources.java     |  6 ++++--
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/modules/common/src/main/java/org/dcache/qos/DefaultQoSPolicyJsonDeserializer.java b/modules/common/src/main/java/org/dcache/qos/DefaultQoSPolicyJsonDeserializer.java
index 5a68bb01988..a8e2c860fd4 100644
--- a/modules/common/src/main/java/org/dcache/qos/DefaultQoSPolicyJsonDeserializer.java
+++ b/modules/common/src/main/java/org/dcache/qos/DefaultQoSPolicyJsonDeserializer.java
@@ -61,6 +61,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
 
 import java.util.ArrayList;
 import java.util.List;
+import org.dcache.util.TimeUtils;
 import org.json.JSONArray;
 import org.json.JSONObject;
 
@@ -99,7 +100,7 @@ private static QoSPolicy deserializePolicy(JSONObject object) {
     private static QoSState deserializeState(JSONObject jsonState) {
         QoSState state = new QoSState();
         if (jsonState.has(DURATION)) {
-            state.setDuration(jsonState.getString(DURATION));
+            state.setDuration(TimeUtils.validateDuration(jsonState.getString(DURATION)));
         }
         List<QoSStorageMediumSpecification> media = new ArrayList<>();
         JSONArray mediaArray = jsonState.getJSONArray(MEDIA);
diff --git a/modules/common/src/main/java/org/dcache/util/TimeUtils.java b/modules/common/src/main/java/org/dcache/util/TimeUtils.java
index c4740bc20ea..ec6c997e84c 100644
--- a/modules/common/src/main/java/org/dcache/util/TimeUtils.java
+++ b/modules/common/src/main/java/org/dcache/util/TimeUtils.java
@@ -15,6 +15,7 @@
 import java.text.SimpleDateFormat;
 import java.time.Duration;
 import java.time.Instant;
+import java.time.format.DateTimeParseException;
 import java.time.temporal.ChronoUnit;
 import java.util.Arrays;
 import java.util.Comparator;
@@ -36,6 +37,10 @@ public class TimeUtils {
 
     public static final String TIMESTAMP_FORMAT = "yyyy-MM-dd' 'HH:mm:ss.SSS";
 
+    private static final String BAD_DURATION_FORMAT_ERROR = "Duration '%s' cannot be parsed. "
+          + "Accepted duration is a text string such as 'PnDTnHnMn.nS' based on the ISO-8601 "
+          + "duration format. E.g. 'P30DT2H30M15.5S' for 30 days, 2 hours, 30 minutes, 15.5 seconds.";
+
     /**
      * <p>Compares time units such that the larger unit is
      * ordered before the smaller.</p>
@@ -579,4 +584,13 @@ public static long getMillis(Properties properties, String key) {
     public static Duration durationOf(long value, TimeUnit unit) {
         return Duration.of(value, unit.toChronoUnit());
     }
+
+    public static String validateDuration(String duration) throws IllegalArgumentException {
+        try {
+            Duration.parse(duration);
+            return duration;
+        } catch (DateTimeParseException e) {
+            throw new IllegalArgumentException(String.format(BAD_DURATION_FORMAT_ERROR, duration));
+        }
+    }
 }
diff --git a/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/tape/StageResources.java b/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/tape/StageResources.java
index 0d4c12e6996..3bb947d3715 100644
--- a/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/tape/StageResources.java
+++ b/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/tape/StageResources.java
@@ -106,6 +106,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
 import org.dcache.services.bulk.BulkRequestMessage;
 import org.dcache.services.bulk.BulkRequestStatusMessage;
 import org.dcache.services.bulk.BulkRequestTargetInfo;
+import org.dcache.util.TimeUtils;
 import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
@@ -381,7 +382,8 @@ private BulkRequest toBulkRequest(String requestPayload) {
                 String path = file.getString("path");
                 paths.add(path);
                 if (file.has("diskLifetime")) {
-                    jsonLifetimes.put(path, file.getString("diskLifetime"));
+                    jsonLifetimes.put(path,
+                          TimeUtils.validateDuration(file.getString("diskLifetime")));
                 }
                 if (file.has("targetedMetadata")) {
                     getTargetedMetadataForPath(file).ifPresent(mdata ->
@@ -394,7 +396,7 @@ private BulkRequest toBulkRequest(String requestPayload) {
             arguments.put("diskLifetime", jsonLifetimes.toString());
             arguments.put("targetedMetadata", jsonMetadata.toString());
             request.setArguments(arguments);
-        } catch (JSONException e) {
+        } catch (JSONException | IllegalArgumentException e) {
             throw newBadRequestException(requestPayload, e);
         }