Skip to content

Commit

Permalink
dcache-frontend,common: check parsing of Duration in STAGE
Browse files Browse the repository at this point in the history
Motivation:

See #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
  • Loading branch information
alrossi authored and mksahakyan committed Nov 17, 2023
1 parent 2e559af commit 9d3295b
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 2 deletions.
14 changes: 14 additions & 0 deletions modules/common/src/main/java/org/dcache/util/TimeUtils.java
Expand Up @@ -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;
Expand All @@ -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>
Expand Down Expand Up @@ -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));
}
}
}
Expand Up @@ -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;
Expand Down Expand Up @@ -382,7 +383,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 ->
Expand All @@ -395,7 +397,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);
}

Expand Down

0 comments on commit 9d3295b

Please sign in to comment.