Merge pull request #800 from molgenis/fix/779-empty-linkfile-name

fix: #799 empty linkfile name
molgenis · Oct 17, 2024 · a3bcf5e · a3bcf5e
2 parents 8489363 + db092b2
commit a3bcf5e
Show file tree

Hide file tree

Showing 5 changed files with 111 additions and 18 deletions.
diff --git a/armadillo/src/main/java/org/molgenis/armadillo/exceptions/InvalidObjectNameException.java b/armadillo/src/main/java/org/molgenis/armadillo/exceptions/InvalidObjectNameException.java
@@ -0,0 +1,17 @@
+package org.molgenis.armadillo.exceptions;
+
+import static java.lang.String.format;
+import static org.springframework.http.HttpStatus.BAD_REQUEST;
+
+import org.springframework.web.bind.annotation.ResponseStatus;
+
+@ResponseStatus(BAD_REQUEST)
+public class InvalidObjectNameException extends RuntimeException {
+
+    public InvalidObjectNameException(String objectName) {
+        super(
+                format(
+                        "Object name '%s' is invalid. Object format should be in the following format: folder/file.",
+                        objectName));
+    }
+}
diff --git a/armadillo/src/main/java/org/molgenis/armadillo/storage/ArmadilloLinkFile.java b/armadillo/src/main/java/org/molgenis/armadillo/storage/ArmadilloLinkFile.java
@@ -30,6 +30,10 @@ public ArmadilloLinkFile(
       String variables,
       String linkObject,
       String project) {
+    Boolean linkObjectIsValid = isValidLinkObject(linkObject);
+    if (!linkObjectIsValid) {
+      throw new IllegalArgumentException(format("Invalid link object: %s", linkObject));
+    }
     this.linkObject = linkObject;
     this.sourceProject = sourceProject;
     this.sourceObject = sourceObject;
@@ -70,6 +74,23 @@ public ArmadilloLinkFile(InputStream armadilloLinkStream, String linkProject, St
     }
   }
 
+  static Boolean isValidLinkObject(String linkObject) {
+    // If object name ends or starts with / it means folder or filename is empty
+    if (linkObject.endsWith("/") || linkObject.startsWith("/")) {
+      return false;
+    }
+    int count = 0;
+    for (int i = 0; i < linkObject.length(); i++) {
+      if (linkObject.charAt(i) == '/') {
+        count++;
+        if (count > 1) {
+          return false;
+        }
+      }
+    }
+    return count == 1;
+  }
+
   public String getSourceProject() {
     return this.sourceProject;
   }

diff --git a/armadillo/src/main/java/org/molgenis/armadillo/storage/ArmadilloStorageService.java b/armadillo/src/main/java/org/molgenis/armadillo/storage/ArmadilloStorageService.java
@@ -90,7 +90,7 @@ public void createLinkedObject(
       String linkName,
       String linkProject,
       String variables)
-      throws IOException {
+      throws IOException, StorageException {
     throwIfUnknown(sourceProject, sourceObject + PARQUET);
     throwIfUnknown(linkProject);
     throwIfDuplicate(linkProject, linkName + LINK_FILE);
@@ -102,11 +102,15 @@ public void createLinkedObject(
       throw new UnknownVariableException(
           sourceProject, sourceObject, unavailableVariables.toString());
     }
-    ArmadilloLinkFile armadilloLinkFile =
-        createLinkFileFromSource(sourceProject, sourceObject, variables, linkName, linkProject);
-    InputStream is = armadilloLinkFile.toStream();
-    storageService.save(
-        is, SHARED_PREFIX + linkProject, armadilloLinkFile.getFileName(), APPLICATION_JSON);
+    try {
+      ArmadilloLinkFile armadilloLinkFile =
+              createLinkFileFromSource(sourceProject, sourceObject, variables, linkName, linkProject);
+      InputStream is = armadilloLinkFile.toStream();
+      storageService.save(
+              is, SHARED_PREFIX + linkProject, armadilloLinkFile.getFileName(), APPLICATION_JSON);
+    } catch (IllegalArgumentException e) {
+      throw new InvalidObjectNameException(linkName);
+    }
   }
 
   public ArmadilloLinkFile createArmadilloLinkFileFromStream(
@@ -223,7 +227,7 @@ private static Workspace toWorkspace(ObjectMetadata item) {
         .build();
   }
 
-  private void trySaveWorkspace (ArmadilloWorkspace workspace, Principal principal, String id) {
+  private void trySaveWorkspace(ArmadilloWorkspace workspace, Principal principal, String id) {
     try {
       storageService.save(
               workspace.createInputStream(),

diff --git a/armadillo/src/test/java/org/molgenis/armadillo/storage/ArmadilloLinkFileTest.java b/armadillo/src/test/java/org/molgenis/armadillo/storage/ArmadilloLinkFileTest.java
@@ -1,7 +1,8 @@
 package org.molgenis.armadillo.storage;
 
 import static java.lang.String.format;
-import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.*;
+import static org.molgenis.armadillo.storage.ArmadilloLinkFile.isValidLinkObject;
 
 import com.google.gson.JsonObject;
 import com.google.gson.JsonParseException;
@@ -20,41 +21,49 @@ public class ArmadilloLinkFileTest {
   String linkProject = "view-project";
   ArmadilloLinkFile alf = new ArmadilloLinkFile(srcProject, srcObj, vars, linkObj, linkProject);
 
+  @Test void testThrowsIllegalArgumentExceptionWhenInvalidLinkObject() {
+    try {
+      new ArmadilloLinkFile(srcProject, srcObj, vars, "broken", linkProject);
+    } catch (IllegalArgumentException e) {
+      assertEquals("Invalid link object: broken", e.getMessage());
+    }
+  }
+
   @Test
-  public void testBuildJson() {
+  void testBuildJson() {
     JsonObject actual = alf.buildJson();
     assertEquals(srcObj, actual.get("sourceObject").getAsString());
     assertEquals(srcProject, actual.get("sourceProject").getAsString());
     assertEquals(vars, actual.get("variables").getAsString());
   }
 
   @Test
-  public void testToString() {
+  void testToString() {
     String actual = alf.toString();
     assertEquals(testData, actual);
   }
 
   @Test
-  public void testGetFileName() {
+  void testGetFileName() {
     String actual = alf.getFileName();
     assertEquals("folder/link-obj.alf", actual);
   }
 
   @Test
-  public void testGetNumberOfVariables() {
+  void testGetNumberOfVariables() {
     Integer actual = alf.getNumberOfVariables();
     assertEquals(3, actual);
   }
 
   @Test
-  public void testLoadFromStream() {
+  void testLoadFromStream() {
     InputStream inputStream = new ByteArrayInputStream(testData.getBytes());
     JsonObject actual = alf.loadFromStream(inputStream);
     assertEquals(testData, actual.toString());
   }
 
   @Test
-  public void testLoadLinkFileFromStream() {
+  void testLoadLinkFileFromStream() {
     InputStream inputStream = new ByteArrayInputStream(testData.getBytes());
     ArmadilloLinkFile alfFromStream = new ArmadilloLinkFile(inputStream, linkObj, linkProject);
     JsonObject jsonFromStream = alfFromStream.buildJson();
@@ -64,7 +73,7 @@ public void testLoadLinkFileFromStream() {
   }
 
   @Test
-  public void testLoadLinkFileFromStreamInvalidJson() {
+  void testLoadLinkFileFromStreamInvalidJson() {
     String testData =
         "\"sourceObject\":\"folder/src-obj\",\"sourceProject\":\"src-project\",\"variables\":\"var1,var2,var3\"}";
     InputStream inputStream = new ByteArrayInputStream(testData.getBytes());
@@ -77,7 +86,7 @@ public void testLoadLinkFileFromStreamInvalidJson() {
   }
 
   @Test
-  public void testLoadLinkFileFromStreamInvalidObj() {
+  void testLoadLinkFileFromStreamInvalidObj() {
     String testData =
         "{\"sourceObj\":\"folder/src-obj\",\"sourceProject\":\"src-project\",\"variables\":\"var1,var2,var3\"}";
     InputStream inputStream = new ByteArrayInputStream(testData.getBytes());
@@ -90,7 +99,7 @@ public void testLoadLinkFileFromStreamInvalidObj() {
   }
 
   @Test
-  public void testLoadLinkFileFromStreamInvalidProject() {
+  void testLoadLinkFileFromStreamInvalidProject() {
     String testData =
         "{\"sourceObject\":\"folder/src-obj\",\"sourceProj\":\"src-project\",\"variables\":\"var1,var2,var3\"}";
     InputStream inputStream = new ByteArrayInputStream(testData.getBytes());
@@ -103,7 +112,37 @@ public void testLoadLinkFileFromStreamInvalidProject() {
   }
 
   @Test
-  public void testLoadLinkFileFromStreamInvalidVariables() {
+  void testIsValidLinkObjectTooManySlashes() {
+    String tooManySlashes = "/this/is/too/many";
+    assertFalse(isValidLinkObject(tooManySlashes));
+  }
+
+  @Test
+  void testIsValidLinkObjectEndsWithSlash() {
+    String endsWithSlash = "endswith/";
+    assertFalse(isValidLinkObject(endsWithSlash));
+  }
+
+  @Test
+  void testIsValidLinkObjectStartsWithSlash() {
+    String startsWithSlash = "/startswith";
+    assertFalse(isValidLinkObject(startsWithSlash));
+  }
+
+  @Test
+  void testIsValidLinkObjectNotEnoughSlashes() {
+    String noSlash = "no slash";
+    assertFalse(isValidLinkObject(noSlash));
+  }
+
+  @Test
+  void testIsValidLinkObjectSucceeds() {
+    String object = "folder/file";
+    assertTrue(isValidLinkObject(object));
+  }
+
+  @Test
+  void testLoadLinkFileFromStreamInvalidVariables() {
     String testData =
         "{\"sourceObject\":\"folder/src-obj\",\"sourceProject\":\"src-project\",\"variabls\":\"var1,var2,var3\"}";
     InputStream inputStream = new ByteArrayInputStream(testData.getBytes());

diff --git a/armadillo/src/test/java/org/molgenis/armadillo/storage/ArmadilloStorageServiceTest.java b/armadillo/src/test/java/org/molgenis/armadillo/storage/ArmadilloStorageServiceTest.java
@@ -611,6 +611,18 @@ void testCreateLinkedObjectUnknownSrcObj() {
                 "gecko", "1_0_release_1_1/gecko", "folder/my_link", "diabetes", "a,b,c"));
   }
 
+  @Test
+  @WithMockUser(roles = "SU")
+  void testCreateLinkedObjecInvalidLinkObj() {
+    mockExistingObject(SHARED_GECKO, "1_0_release_1_1/gecko.parquet");
+    when(storageService.listBuckets()).thenReturn(List.of(SHARED_DIABETES, SHARED_GECKO));
+    assertThrows(
+            InvalidObjectNameException.class,
+            () ->
+                    armadilloStorage.createLinkedObject(
+                            "gecko", "1_0_release_1_1/gecko", "my_link", "diabetes", "a,b,c"));
+  }
+
   @Test
   @WithMockUser(roles = "SU")
   void testCreateLinkedObjectUnknownSrcProject() {