Skip to content

Commit

Permalink
dcache-frontend: handle parse exceptions in bulk and tape resources
Browse files Browse the repository at this point in the history 
Motivation:

JSONException and the various exceptions thrown by
GSON are RuntimeExceptions; if they are not caught
and converted into HTTP exceptions, they generate
stack traces.

Modification:

Wrap the relevant code with try ... catch blocks.

Result:

No stack traces when  badly formed objects are
sent by users.

Target: master
Request: 8.2
Patch: https://rb.dcache.org/r/13816/
Requires-notes: yes
Acked-by: Lea
  • Loading branch information
@alrossi @lemora
alrossi authored and lemora committed Dec 14, 2022
1 parent 8199c72 commit 57ef15f
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 53 deletions.
13 changes: 11 additions & 2 deletions modules/dcache-frontend/src/main/java/org/dcache/restful/resources/bulk/BulkResources.java
View file
Expand Up @@ -63,6 +63,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.gson.Gson;
import com.google.gson.JsonParseException;
import io.swagger.annotations.Api;
import io.swagger.annotations.ApiOperation;
import io.swagger.annotations.ApiParam;
Expand Down Expand Up @@ -405,7 +406,14 @@ static BulkRequest toBulkRequest(String requestPayload) {
throw new BadRequestException("empty request payload.");
}

Map map = new Gson().fromJson(requestPayload, Map.class);
Map map;
try {
map = new Gson().fromJson(requestPayload, Map.class);
} catch (JsonParseException e) {
throw new BadRequestException(
String.format("badly formed json object (%s): %s.", requestPayload, e));
}

BulkRequest request = new BulkRequest();

Map<String, Object> arguments = (Map<String, Object>) map.remove("arguments");
Expand All @@ -425,7 +433,8 @@ static BulkRequest toBulkRequest(String requestPayload) {
String string = removeEntry(map, String.class, "activity");
request.setActivity(string);

string = removeEntry(map, String.class, "target_prefix", "target-prefix", "targetPrefix");
string = removeEntry(map, String.class, "target_prefix", "target-prefix",
"targetPrefix");
request.setTargetPrefix(string);

string = removeEntry(map, String.class, "expand_directories", "expand-directories",
Expand Down
28 changes: 19 additions & 9 deletions ...dcache-frontend/src/main/java/org/dcache/restful/resources/tape/ArchiveInfoResources.java
View file
Expand Up @@ -82,6 +82,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import org.dcache.restful.util.HandlerBuilders;
import org.dcache.restful.util.wlcg.ArchiveInfoCollector;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
import org.springframework.stereotype.Component;

Expand Down Expand Up @@ -128,17 +129,26 @@ public List<ArchiveInfo> getArchiveInfo(
@ApiParam(value = "List of paths for which to return archive info (file locality).",
required = true)
String requestPayload) {
JSONObject jsonPayload = new JSONObject(requestPayload);
if (!jsonPayload.has("paths")) {
throw new BadRequestException("request had no paths.");
}

JSONArray jsonArray = jsonPayload.getJSONArray("paths");
int len = Math.min(jsonArray.length(), archiveInfoCollector.getMaxPaths());
List<String> paths;

try {
JSONObject jsonPayload = new JSONObject(requestPayload);

if (!jsonPayload.has("paths")) {
throw new BadRequestException("request had no paths.");
}

JSONArray jsonArray = jsonPayload.getJSONArray("paths");
int len = Math.min(jsonArray.length(), archiveInfoCollector.getMaxPaths());

List<String> paths = new ArrayList<>();
for (int i = 0; i < len; ++i) {
paths.add(jsonArray.getString(i));
paths = new ArrayList<>();
for (int i = 0; i < len; ++i) {
paths.add(jsonArray.getString(i));
}
} catch (JSONException e) {
throw new BadRequestException(
String.format("badly formed json object (%s): %s.", requestPayload, e));
}

return archiveInfoCollector.getInfo(HandlerBuilders.roleAwarePnfsHandler(pnfsManager),
Expand Down
30 changes: 20 additions & 10 deletions ...les/dcache-frontend/src/main/java/org/dcache/restful/resources/tape/ReleaseResources.java
View file
Expand Up @@ -90,6 +90,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import org.dcache.services.bulk.BulkRequest.Depth;
import org.dcache.services.bulk.BulkRequestMessage;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
import org.springframework.stereotype.Component;

Expand Down Expand Up @@ -136,16 +137,25 @@ public Response release(
+ "stage request corresponding to the id, this request will fail.", required = true)
String requestPayload) {

JSONObject reqPayload = new JSONObject(requestPayload);
JSONArray paths = reqPayload.getJSONArray("paths");
if (paths == null) {
throw new BadRequestException("release request contains no paths.");
}

int len = paths.length();
List<String> targetPaths = new ArrayList<>();
for (int i = 0; i < len; ++i) {
targetPaths.add(paths.getString(i));
JSONArray paths;
List<String> targetPaths;

try {
JSONObject reqPayload = new JSONObject(requestPayload);
paths = reqPayload.getJSONArray("paths");

if (paths == null) {
throw new BadRequestException("release request contains no paths.");
}

int len = paths.length();
targetPaths = new ArrayList<>();
for (int i = 0; i < len; ++i) {
targetPaths.add(paths.getString(i));
}
} catch (JSONException e) {
throw new BadRequestException(
String.format("badly formed json object (%s): %s.", requestPayload, e));
}

Subject subject = getSubject();
Expand Down
71 changes: 39 additions & 32 deletions modules/dcache-frontend/src/main/java/org/dcache/restful/resources/tape/StageResources.java
View file
Expand Up @@ -101,6 +101,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import org.dcache.services.bulk.BulkRequestStatusMessage;
import org.dcache.services.bulk.BulkRequestTargetInfo;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
import org.springframework.beans.factory.annotation.Required;
import org.springframework.stereotype.Component;
Expand Down Expand Up @@ -328,43 +329,49 @@ private BulkRequest toBulkRequest(String requestPayload) {
request.setDelayClear(0);
request.setActivity("STAGE");

JSONObject reqPayload = new JSONObject(requestPayload);

JSONArray fileset = reqPayload.getJSONArray("files");
if (fileset == null || fileset.length() == 0) {
throw new BadRequestException("request contains no files.");
}
reqPayload.remove("files");

if (reqPayload.length() != 0) {
throw new BadRequestException("unrecognized payload element(s): " + reqPayload.names());
}

List<String> paths = new ArrayList<>();
JSONObject jsonLifetimes = new JSONObject();
JSONObject jsonMetadata = new JSONObject();
try {
JSONObject reqPayload = new JSONObject(requestPayload);

int len = fileset.length();
for (int i = 0; i < len; ++i) {
JSONObject file = fileset.getJSONObject(i);
if (!file.has("path")) {
throw new BadRequestException("file object " + i + " has no path.");
JSONArray fileset = reqPayload.getJSONArray("files");
if (fileset == null || fileset.length() == 0) {
throw new BadRequestException("request contains no files.");
}
String path = file.getString("path");
paths.add(path);
if (file.has("diskLifetime")) {
jsonLifetimes.put(path, file.getString("diskLifetime"));
reqPayload.remove("files");

if (reqPayload.length() != 0) {
throw new BadRequestException(
"unrecognized payload element(s): " + reqPayload.names());
}
if (file.has("targetedMetadata")) {
getTargetedMetadataForPath(file).ifPresent(mdata ->
jsonMetadata.put(path, mdata.toString()));

List<String> paths = new ArrayList<>();
JSONObject jsonLifetimes = new JSONObject();
JSONObject jsonMetadata = new JSONObject();

int len = fileset.length();
for (int i = 0; i < len; ++i) {
JSONObject file = fileset.getJSONObject(i);
if (!file.has("path")) {
throw new BadRequestException("file object " + i + " has no path.");
}
String path = file.getString("path");
paths.add(path);
if (file.has("diskLifetime")) {
jsonLifetimes.put(path, file.getString("diskLifetime"));
}
if (file.has("targetedMetadata")) {
getTargetedMetadataForPath(file).ifPresent(mdata ->
jsonMetadata.put(path, mdata.toString()));
}
}
}

request.setTarget(paths);
Map<String, String> arguments = new HashMap<>();
arguments.put("diskLifetime", jsonLifetimes.toString());
arguments.put("targetedMetadata", jsonMetadata.toString());
request.setTarget(paths);
Map<String, String> arguments = new HashMap<>();
arguments.put("diskLifetime", jsonLifetimes.toString());
arguments.put("targetedMetadata", jsonMetadata.toString());
} catch (JSONException e) {
throw new BadRequestException(
String.format("badly formed json object (%s): %s.", requestPayload, e));
}

return request;
}
Expand Down

0 comments on commit 57ef15f

Please sign in to comment.