[ZEPPELIN-6692] centralize folder path handling

zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/NotebookPathValidator.java

@@ -22,7 +22,7 @@
 import java.util.regex.Pattern;
 
 /**
- * Note-path validation helpers shared by {@link NotebookRepo} implementations
+ * Notebook path validation helpers shared by {@link NotebookRepo} implementations
  * and the service layer. A {@code final} class with {@code static} methods
  * (rather than {@link NotebookRepo} default methods) prevents an
  * implementation from accidentally — or intentionally — overriding the
@@ -84,4 +84,44 @@ public static String decodeRepeatedly(String encoded) throws IOException {
     }
     throw new IOException("Exceeded maximum decode attempts. Possible malicious input.");
   }
+
+  /**
+   * Normalizes a path using the rules shared by note and folder paths.
+   *
+   * @param path the path to normalize
+   * @return the normalized path
+   * @throws IOException if the path cannot be normalized
+   */
+  public static String normalizePath(String path) throws IOException {
+    if (path == null) {
+      throw new IOException("Path must not be null");
+    }
+
+    if (!path.startsWith("/")) {
+      path = "/" + path;
+    }
+
+    path = decodeRepeatedly(path);
+
+    if (path.contains("..")) {
+      throw new IOException("Path can not contain '..'");
+    }
+
+    return path;
+  }
+
+  /**
+   * Requires {@code folderPath} to use the canonical absolute folder-path form.
+   *
+   * @throws IOException if the path is null or does not start with {@code /}
+   */
+  public static void requireAbsoluteFolderPath(String folderPath) throws IOException {
+    if (folderPath == null) {
+      throw new IOException("Folder path must not be null");
+    }
+
+    if (!folderPath.startsWith("/")) {
+      throw new IOException("Folder path must start with '/'");
+    }
+  }
 }

zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java

@@ -197,6 +197,9 @@ public void move(String noteId,
   @Override
   public void move(String folderPath, String newFolderPath,
                    AuthenticationInfo subject) throws IOException{
+    NotebookPathValidator.requireAbsoluteFolderPath(folderPath);
+    NotebookPathValidator.requireAbsoluteFolderPath(newFolderPath);
+
     LOGGER.info("Move folder from {} to {}", folderPath, newFolderPath);
     FileObject fileObject = rootNotebookFileObject.resolveFile(
         folderPath.substring(1), NameScope.DESCENDENT);
@@ -218,6 +221,8 @@ public void remove(String noteId, String notePath, AuthenticationInfo subject)
 
   @Override
   public void remove(String folderPath, AuthenticationInfo subject) throws IOException {
+    NotebookPathValidator.requireAbsoluteFolderPath(folderPath);
+
     LOGGER.info("Remove folder: {}", folderPath);
     FileObject folderObject = rootNotebookFileObject.resolveFile(
         folderPath.substring(1), NameScope.DESCENDENT);

zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java

@@ -234,13 +234,11 @@ String normalizeNotePath(String notePath) throws IOException {
     if (StringUtils.isBlank(notePath)) {
       notePath = "/Untitled Note";
     }
-    if (!notePath.startsWith("/")) {
-      notePath = "/" + notePath;
-    }
 
     notePath = notePath.replace("\r", " ").replace("\n", " ");
 
-    notePath = NotebookPathValidator.decodeRepeatedly(notePath);
+    notePath = NotebookPathValidator.normalizePath(notePath);
+
     if (notePath.endsWith("/")) {
       throw new IOException("Note name shouldn't end with '/'");
     }
@@ -250,12 +248,21 @@ String normalizeNotePath(String notePath) throws IOException {
       throw new IOException("Note name must be less than 255");
     }
 
-    if (notePath.contains("..")) {
-      throw new IOException("Note name can not contain '..'");
-    }
     return notePath;
   }
 
+  /**
+   * Normalizes a folder path to the canonical absolute form used by folder operations.
+   * Accepts paths with or without a leading slash.
+   *
+   * @param folderPath
+   * @return
+   * @throws IOException
+   */
+  String normalizeFolderPath(String folderPath) throws IOException {
+    return NotebookPathValidator.normalizePath(folderPath);
+  }
+
   public void removeNote(String noteId,
                          ServiceContext context,
                          ServiceCallback<String> callback) throws IOException {
@@ -735,6 +742,8 @@ public void restoreFolder(String folderPath,
                             ServiceContext context,
                             ServiceCallback<Void> callback) throws IOException {
 
+    folderPath = normalizeFolderPath(folderPath);
+
     if (!folderPath.startsWith("/" + NoteManager.TRASH_FOLDER)) {
       callback.onFailure(new IOException("Can not restore this folder: " + folderPath +
           " as it is not in trash folder"), context);
@@ -1291,16 +1300,17 @@ public void moveFolderToTrash(String folderPath,
                               ServiceCallback<Void> callback) throws IOException {
 
     //TODO(zjffdu) folder permission check
-    //TODO(zjffdu) folderPath is relative path, need to fix it in frontend
     LOGGER.info("Move folder {} to trash", folderPath);
 
-    String destFolderPath = "/" + NoteManager.TRASH_FOLDER + "/" + folderPath;
+    folderPath = normalizeFolderPath(folderPath);
+
+    String destFolderPath = "/" + NoteManager.TRASH_FOLDER + folderPath;
     if (notebook.containsNote(destFolderPath)) {
       destFolderPath = destFolderPath + " " +
           TRASH_CONFLICT_TIMESTAMP_FORMATTER.format(Instant.now());
     }
 
-    notebook.moveFolder("/" + folderPath, destFolderPath, context.getAutheInfo());
+    notebook.moveFolder(folderPath, destFolderPath, context.getAutheInfo());
     callback.onSuccess(null, context);
   }
 
@@ -1320,7 +1330,7 @@ public List<NoteInfo> removeFolder(String folderPath,
                            ServiceContext context,
                            ServiceCallback<List<NoteInfo>> callback) throws IOException {
     try {
-      notebook.removeFolder(folderPath, context.getAutheInfo());
+      notebook.removeFolder(normalizeFolderPath(folderPath), context.getAutheInfo());
       List<NoteInfo> notesInfo = notebook.getNotesInfo(
               noteId -> authorizationService.isReader(noteId, context.getUserAndRoles()));
       callback.onSuccess(notesInfo, context);
@@ -1338,8 +1348,8 @@ public List<NoteInfo> renameFolder(String folderPath,
     //TODO(zjffdu) folder permission check
 
     try {
-      notebook.moveFolder(normalizeNotePath(folderPath),
-              normalizeNotePath(newFolderPath), context.getAutheInfo());
+      notebook.moveFolder(normalizeFolderPath(folderPath),
+              normalizeFolderPath(newFolderPath), context.getAutheInfo());
       List<NoteInfo> notesInfo = notebook.getNotesInfo(
               noteId -> authorizationService.isReader(noteId, context.getUserAndRoles()));
       callback.onSuccess(notesInfo, context);

zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java

@@ -1061,7 +1061,6 @@ public void onSuccess(String message, ServiceContext context) throws IOException
   private void removeFolder(NotebookSocket conn, ServiceContext context, Message fromMessage) throws IOException {
 
     String folderPath = (String) fromMessage.get("id");
-    folderPath = "/" + folderPath;
     getNotebookService().removeFolder(folderPath, context,
         new WebSocketServiceCallback<List<NoteInfo>>(conn) {
           @Override
@@ -1121,7 +1120,6 @@ private void restoreFolder(NotebookSocket conn,
                              ServiceContext context,
                              Message fromMessage) throws IOException {
     String folderPath = (String) fromMessage.get("id");
-    folderPath = "/" + folderPath;
     getNotebookService().restoreFolder(folderPath, context,
         new WebSocketServiceCallback<Void>(conn) {
           @Override

zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoPathValidationTest.java

@@ -16,6 +16,7 @@
  */
 package org.apache.zeppelin.notebook.repo;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
@@ -110,4 +111,65 @@ void decodeRepeatedly_accepts_max_decode_layers() throws IOException {
     // cleanly; the constant means *layers*, not raw loop iterations.
     assertEquals("/..", NotebookPathValidator.decodeRepeatedly("/%252525252e%252525252e"));
   }
+
+  @Test
+  void normalizePath_adds_leading_slash() throws IOException {
+    assertEquals("/folder/note", NotebookPathValidator.normalizePath("folder/note"));
+  }
+
+  @Test
+  void normalizePath_keeps_existing_leading_slash() throws IOException {
+    assertEquals("/folder/note", NotebookPathValidator.normalizePath("/folder/note"));
+  }
+
+  @Test
+  void normalizePath_decodes_url_encoding() throws IOException {
+    assertEquals("/folder/My Note", NotebookPathValidator.normalizePath("/folder/My%20Note"));
+  }
+
+  @Test
+  void normalizePath_decodes_repeated_url_encoding() throws IOException {
+    assertEquals("/folder/My Note", NotebookPathValidator.normalizePath("/folder/My%2520Note"));
+  }
+
+  @ParameterizedTest
+  @ValueSource(strings = {
+      "/foo/../bar",
+      "/foo..bar",
+      "/...",
+      "/%2e%2e/bar",
+      "/%252e%252e/bar"
+  })
+  void normalizePath_rejects_double_dot(String path) {
+    assertThrows(IOException.class, () -> NotebookPathValidator.normalizePath(path));
+  }
+
+  @Test
+  void normalizePath_rejects_null() {
+    assertThrows(IOException.class, () -> NotebookPathValidator.normalizePath(null));
+  }
+
+  @Test
+  void requireAbsoluteFolderPath_accepts_absolute_path() {
+    assertDoesNotThrow(
+        () -> NotebookPathValidator.requireAbsoluteFolderPath("/folder/subfolder"));
+  }
+
+  @Test
+  void requireAbsoluteFolderPath_accepts_root_path() {
+    assertDoesNotThrow(
+        () -> NotebookPathValidator.requireAbsoluteFolderPath("/"));
+  }
+
+  @Test
+  void requireAbsoluteFolderPath_rejects_relative_path() {
+    assertThrows(IOException.class,
+        () -> NotebookPathValidator.requireAbsoluteFolderPath("folder/subfolder"));
+  }
+
+  @Test
+  void requireAbsoluteFolderPath_rejects_null() {
+    assertThrows(IOException.class,
+        () -> NotebookPathValidator.requireAbsoluteFolderPath(null));
+  }
 }

zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java

@@ -39,6 +39,7 @@
 import java.util.Map;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 class VFSNotebookRepoTest {
 
@@ -177,4 +178,18 @@ private void createNewDirectory(String dirName) {
     File dir = new File(notebookRepo.rootNotebookFolder + "/" + dirName);
     dir.mkdir();
   }
+
+  @Test
+  void testMoveFolderRequiresAbsolutePath() {
+    assertThrows(IOException.class,
+        () -> notebookRepo.move("my_project", "/new_project", AuthenticationInfo.ANONYMOUS));
+    assertThrows(IOException.class,
+        () -> notebookRepo.move("/my_project", "new_project", AuthenticationInfo.ANONYMOUS));
+  }
+
+  @Test
+  void testRemoveFolderRequiresAbsolutePath(){
+    assertThrows(IOException.class,
+        () -> notebookRepo.remove("my_project", AuthenticationInfo.ANONYMOUS));
+  }
 }

zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java

@@ -796,20 +796,20 @@ void testNormalizeNotePath() throws IOException {
       notebookService.normalizeNotePath("my..note");
       fail("Should fail");
     } catch (IOException e) {
-      assertEquals("Note name can not contain '..'", e.getMessage());
+      assertEquals("Path can not contain '..'", e.getMessage());
     }
     try {
       notebookService.normalizeNotePath("%2e%2e/%2e%2e/tmp/test222");
       fail("Should fail");
     } catch (IOException e) {
-      assertEquals("Note name can not contain '..'", e.getMessage());
+      assertEquals("Path can not contain '..'", e.getMessage());
     }
     try {
       // Double URL encoding of ".."
       notebookService.normalizeNotePath("%252e%252e/%252e%252e/tmp/test333");
       fail("Should fail");
     } catch (IOException e) {
-      assertEquals("Note name can not contain '..'", e.getMessage());
+      assertEquals("Path can not contain '..'", e.getMessage());
     }
     try {
       notebookService.normalizeNotePath("%25252525252e%25252525252e/tmp/test444");
@@ -824,4 +824,14 @@ void testNormalizeNotePath() throws IOException {
       assertEquals("Note name shouldn't end with '/'", e.getMessage());
     }
   }
+
+  @Test
+  void testNormalizeFolderPath() throws IOException {
+    assertEquals("/folder", notebookService.normalizeFolderPath("folder"));
+    assertEquals("/folder", notebookService.normalizeFolderPath("/folder"));
+    assertEquals("/folder/subfolder", notebookService.normalizeFolderPath("folder/subfolder"));
+    assertEquals("/folder/subfolder", notebookService.normalizeFolderPath("/folder/subfolder"));
+
+    assertThrows(IOException.class, () -> notebookService.normalizeFolderPath(null));
+  }
 }