(control) Add basic input validation to node actions

Will present a simple error message when required fields aren't populated, instead of a cryptic HTTP status error.

code/api/executor-api/src/main/java/nu/marginalia/executor/client/ExecutorClient.java

@@ -14,6 +14,7 @@ import nu.marginalia.executor.upload.UploadDirContents;
 import nu.marginalia.model.gson.GsonFactory;
 import nu.marginalia.service.descriptor.ServiceDescriptors;
 import nu.marginalia.service.id.ServiceId;
+import nu.marginalia.storage.model.FileStorage;
 import nu.marginalia.storage.model.FileStorageId;
 
 import java.io.OutputStream;
@@ -21,7 +22,6 @@ import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
 import java.util.ArrayList;
-import java.util.List;
 import java.util.concurrent.TimeUnit;
 
 public class ExecutorClient extends AbstractDynamicClient {
@@ -97,13 +97,13 @@ public class ExecutorClient extends AbstractDynamicClient {
                 .blockingSubscribe();
     }
 
-    public void exportAtags(Context ctx, int node, String fid) {
+    public void exportAtags(Context ctx, int node, FileStorageId fid) {
         post(ctx, node, "/export/atags?fid="+fid, "").blockingSubscribe();
     }
-    public void exportRssFeeds(Context ctx, int node, String fid) {
+    public void exportRssFeeds(Context ctx, int node, FileStorageId fid) {
         post(ctx, node, "/export/feeds?fid="+fid, "").blockingSubscribe();
     }
-    public void exportTermFrequencies(Context ctx, int node, String fid) {
+    public void exportTermFrequencies(Context ctx, int node, FileStorageId fid) {
         post(ctx, node, "/export/termfreq?fid="+fid, "").blockingSubscribe();
     }
 
@@ -111,7 +111,7 @@ public class ExecutorClient extends AbstractDynamicClient {
         post(ctx, node, "/export/data", "").blockingSubscribe();
     }
 
-    public void restoreBackup(Context context, int node, String fid) {
+    public void restoreBackup(Context context, int node, FileStorageId fid) {
         post(context, node, "/backup/" + fid + "/restore", "").blockingSubscribe();
     }
 

code/services-core/control-service/src/main/java/nu/marginalia/control/node/svc/ControlNodeActionsService.java

@@ -3,6 +3,7 @@ package nu.marginalia.control.node.svc;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import nu.marginalia.client.Context;
+import nu.marginalia.control.ControlValidationError;
 import nu.marginalia.control.RedirectControl;
 import nu.marginalia.executor.client.ExecutorClient;
 import nu.marginalia.executor.model.load.LoadParameters;
@@ -87,12 +88,16 @@ public class ControlNodeActionsService {
         );
     }
 
-    public Object sideloadEncyclopedia(Request request, Response response) throws Exception {
+    public Object sideloadEncyclopedia(Request request, Response response) {
 
-        Path sourcePath = Path.of(request.queryParams("source"));
+        String source = request.queryParams("source");
         String baseUrl = request.queryParams("baseUrl");
+        int nodeId = Integer.parseInt(request.params("node"));
 
-        final int nodeId = Integer.parseInt(request.params("node"));
+        if (baseUrl == null)
+            throw new ControlValidationError("No baseUrl specified", "A baseUrl must be specified", "..");
+
+        Path sourcePath = parseSourcePath(source);
 
         eventLog.logEvent("USER-ACTION", "SIDELOAD ENCYCLOPEDIA " + nodeId);
 
@@ -101,11 +106,12 @@ public class ControlNodeActionsService {
         return "";
     }
 
-    public Object sideloadDirtree(Request request, Response response) throws Exception {
+    public Object sideloadDirtree(Request request, Response response) {
 
-        Path sourcePath = Path.of(request.queryParams("source"));
         final int nodeId = Integer.parseInt(request.params("node"));
 
+        Path sourcePath = parseSourcePath(request.queryParams("source"));
+
         eventLog.logEvent("USER-ACTION", "SIDELOAD DIRTREE " + nodeId);
 
         executorClient.sideloadDirtree(Context.fromRequest(request), nodeId, sourcePath);
@@ -113,10 +119,10 @@ public class ControlNodeActionsService {
         return "";
     }
 
-    public Object sideloadWarc(Request request, Response response) throws Exception {
+    public Object sideloadWarc(Request request, Response response) {
 
-        Path sourcePath = Path.of(request.queryParams("source"));
         final int nodeId = Integer.parseInt(request.params("node"));
+        Path sourcePath = parseSourcePath(request.queryParams("source"));
 
         eventLog.logEvent("USER-ACTION", "SIDELOAD WARC " + nodeId);
 
@@ -124,11 +130,15 @@ public class ControlNodeActionsService {
 
         return "";
     }
-    public Object sideloadStackexchange(Request request, Response response) throws Exception {
+    public Object sideloadStackexchange(Request request, Response response) {
 
-        Path sourcePath = Path.of(request.queryParams("source"));
         final int nodeId = Integer.parseInt(request.params("node"));
 
+        String source = request.queryParams("source");
+        if (source == null)
+            throw new ControlValidationError("No source specified", "A source file/directory must be specified", "..");
+        Path sourcePath = Path.of(source);
+
         eventLog.logEvent("USER-ACTION", "SIDELOAD STACKEXCHANGE " + nodeId);
 
         executorClient.sideloadStackexchange(Context.fromRequest(request), nodeId, sourcePath);
@@ -143,7 +153,8 @@ public class ControlNodeActionsService {
 
     private Object triggerAutoRecrawl(Request request, Response response) throws SQLException {
         int nodeId = Integer.parseInt(request.params("id"));
-        var toCrawl = FileStorageId.parse(request.queryParams("source"));
+
+        var toCrawl = parseSourceFileStorageId(request.queryParams("source"));
 
         changeActiveStorage(nodeId, FileStorageType.CRAWL_DATA, toCrawl);
 
@@ -159,7 +170,7 @@ public class ControlNodeActionsService {
     private Object triggerNewCrawl(Request request, Response response) throws SQLException {
         int nodeId = Integer.parseInt(request.params("id"));
 
-        var toCrawl = FileStorageId.parse(request.queryParams("source"));
+        var toCrawl = parseSourceFileStorageId(request.queryParams("source"));
 
         changeActiveStorage(nodeId, FileStorageType.CRAWL_SPEC, toCrawl);
 
@@ -174,7 +185,8 @@ public class ControlNodeActionsService {
 
     private Object triggerAutoProcess(Request request, Response response) throws SQLException {
         int nodeId = Integer.parseInt(request.params("id"));
-        var toProcess = FileStorageId.parse(request.queryParams("source"));
+
+        var toProcess = parseSourceFileStorageId(request.queryParams("source"));
 
         changeActiveStorage(nodeId, FileStorageType.PROCESSED_DATA, toProcess);
 
@@ -189,6 +201,10 @@ public class ControlNodeActionsService {
         int nodeId = Integer.parseInt(request.params("id"));
         String[] values = request.queryParamsValues("source");
 
+        if (values.length == 0) {
+            throw new ControlValidationError("No source specified", "At least one source storage must be specified", "..");
+        }
+
         List<FileStorageId> ids = Arrays.stream(values).map(FileStorageId::parse).toList();
 
         changeActiveStorage(nodeId, FileStorageType.PROCESSED_DATA, ids.toArray(new FileStorageId[0]));
@@ -204,11 +220,14 @@ public class ControlNodeActionsService {
     private Object triggerRestoreBackup(Request request, Response response) {
         int nodeId = Integer.parseInt(request.params("id"));
 
-        executorClient.restoreBackup(Context.fromRequest(request), nodeId, request.queryParams("source"));
+        var toLoad = parseSourceFileStorageId(request.queryParams("source"));
+
+        executorClient.restoreBackup(Context.fromRequest(request), nodeId, toLoad);
 
         return "";
     }
 
+
     /** Change the active storage for a node of a particular type. */
     private void changeActiveStorage(int nodeId, FileStorageType type, FileStorageId... newActiveStorage) throws SQLException {
         // It is desirable to have the active storage set to reflect which storage was last used
@@ -231,6 +250,10 @@ public class ControlNodeActionsService {
         final String url = request.queryParams("url");
         int nodeId = Integer.parseInt(request.params("id"));
 
+        if (url == null || url.isBlank()) {
+            throw new ControlValidationError("No url specified", "A url must be specified", "..");
+        }
+
         executorClient.createCrawlSpecFromDownload(Context.fromRequest(request), nodeId, description, url);
 
         return "";
@@ -244,21 +267,41 @@ public class ControlNodeActionsService {
 
     private Object exportFromCrawlData(Request req, Response rsp) {
         String exportType = req.queryParams("exportType");
-        String source = req.queryParams("source");
+        FileStorageId source = parseSourceFileStorageId(req.queryParams("source"));
 
-        if (exportType.equals("atags")) {
+        switch (exportType) {
-            executorClient.exportAtags(Context.fromRequest(req), Integer.parseInt(req.params("id")), source);
+            case "atags" -> executorClient.exportAtags(Context.fromRequest(req), Integer.parseInt(req.params("id")), source);
-        }
+            case "rss" -> executorClient.exportRssFeeds(Context.fromRequest(req), Integer.parseInt(req.params("id")), source);
-        else if (exportType.equals("rss")) {
+            case "termFreq" -> executorClient.exportTermFrequencies(Context.fromRequest(req), Integer.parseInt(req.params("id")), source);
-            executorClient.exportRssFeeds(Context.fromRequest(req), Integer.parseInt(req.params("id")), source);
+            default -> throw new ControlValidationError("No export type specified", "An export type must be specified", "..");
-        }
-        else if (exportType.equals("termFreq")) {
-            executorClient.exportTermFrequencies(Context.fromRequest(req), Integer.parseInt(req.params("id")), source);
-        }
-        else {
-            rsp.status(404);
         }
 
         return "";
     }
+
+
+
+    private Path parseSourcePath(String source) {
+        if (source == null) {
+            throw new ControlValidationError("No source specified",
+                    "A source file/directory must be specified",
+                    "..");
+        }
+        return Path.of(source);
+    }
+
+    private FileStorageId parseSourceFileStorageId(String source) {
+        if (source == null) {
+            throw new ControlValidationError("No source specified",
+                    "A source file storage must be specified",
+                    "..");
+        }
+
+        try {
+            return FileStorageId.parse(source);
+        }
+        catch (Exception e) { // Typically NumberFormatException
+            throw new ControlValidationError("Invalid source specified", "The source file storage is invalid", "..");
+        }
+    }
 }

code/services-core/control-service/src/main/java/nu/marginalia/control/sys/svc/DataSetsService.java

@@ -16,15 +16,12 @@ import java.util.Map;
 @Singleton
 public class DataSetsService {
 
-    private final HikariDataSource dataSource;
     private final ControlRendererFactory rendererFactory;
     private final DomainTypes domainTypes;
 
     @Inject
-    public DataSetsService(HikariDataSource dataSource,
+    public DataSetsService(ControlRendererFactory rendererFactory,
-                           ControlRendererFactory rendererFactory,
                            DomainTypes domainTypes) {
-        this.dataSource = dataSource;
         this.rendererFactory = rendererFactory;
         this.domainTypes = domainTypes;
     }