From 33b80098a48055d3e4b95090da9b05e3d4fd7098 Mon Sep 17 00:00:00 2001 From: gyowoo1113 Date: Sat, 5 Sep 2026 14:53:24 +0900 Subject: [PATCH 1/4] [ZEPPELIN-6692] Consolidate folder path normalization --- .../zeppelin/service/NotebookService.java | 35 +++++++++++++++---- .../zeppelin/socket/NotebookServer.java | 2 -- .../zeppelin/service/NotebookServiceTest.java | 10 ++++++ 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java index eacd970eb05..64809735e41 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java @@ -256,6 +256,26 @@ String normalizeNotePath(String notePath) throws IOException { 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 { + if (folderPath == null) { + throw new IOException("Folder path must not be null"); + } + + if (!folderPath.startsWith("/")) { + folderPath = "/" + folderPath; + } + + return folderPath; + } + public void removeNote(String noteId, ServiceContext context, ServiceCallback callback) throws IOException { @@ -735,6 +755,8 @@ public void restoreFolder(String folderPath, ServiceContext context, ServiceCallback 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 +1313,17 @@ public void moveFolderToTrash(String folderPath, ServiceCallback 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 +1343,7 @@ public List removeFolder(String folderPath, ServiceContext context, ServiceCallback> callback) throws IOException { try { - notebook.removeFolder(folderPath, context.getAutheInfo()); + notebook.removeFolder(normalizeFolderPath(folderPath), context.getAutheInfo()); List notesInfo = notebook.getNotesInfo( noteId -> authorizationService.isReader(noteId, context.getUserAndRoles())); callback.onSuccess(notesInfo, context); @@ -1338,8 +1361,8 @@ public List 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 notesInfo = notebook.getNotesInfo( noteId -> authorizationService.isReader(noteId, context.getUserAndRoles())); callback.onSuccess(notesInfo, context); diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java index 6ab2ef711e1..34a4860ae32 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java +++ b/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>(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(conn) { @Override diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java index 1c4f554d50a..bf3d713444a 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java @@ -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)); + } } From a222a66770f766d2f33f524d409ad192a9c4e633 Mon Sep 17 00:00:00 2001 From: gyowoo1113 Date: Sat, 5 Sep 2026 15:35:29 +0900 Subject: [PATCH 2/4] [ZEPPELIN-6692] Add folder path validation to VFS repository --- .../notebook/repo/NotebookPathValidator.java | 17 ++++++++++++++++- .../zeppelin/notebook/repo/VFSNotebookRepo.java | 5 +++++ .../notebook/repo/VFSNotebookRepoTest.java | 15 +++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/NotebookPathValidator.java b/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/NotebookPathValidator.java index 40f754076aa..1879f159bcd 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/NotebookPathValidator.java +++ b/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,19 @@ public static String decodeRepeatedly(String encoded) throws IOException { } throw new IOException("Exceeded maximum decode attempts. Possible malicious input."); } + + /** + * 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 '/'"); + } + } } diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java b/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java index 32e433fff8f..420a1c148a9 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepo.java +++ b/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); diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java index 9bf02c0b3da..c7dcc9bdc44 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java +++ b/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)); + } } From 3b34f0705afb34c6957a82b6609e77a7152f6a37 Mon Sep 17 00:00:00 2001 From: gyowoo1113 Date: Wed, 9 Sep 2026 17:35:32 +0900 Subject: [PATCH 3/4] [ZEPPELIN-6692] Extract shared path normalization for note and folder paths --- .../notebook/repo/NotebookPathValidator.java | 25 +++++++++++++ .../zeppelin/service/NotebookService.java | 19 ++-------- .../repo/NotebookRepoPathValidationTest.java | 37 +++++++++++++++++++ .../zeppelin/service/NotebookServiceTest.java | 6 +-- 4 files changed, 68 insertions(+), 19 deletions(-) diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/NotebookPathValidator.java b/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/NotebookPathValidator.java index 1879f159bcd..5a4aabf4831 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/NotebookPathValidator.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/NotebookPathValidator.java @@ -85,6 +85,31 @@ 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. * diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java index 64809735e41..47986f6f7c4 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java +++ b/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,9 +248,6 @@ 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; } @@ -265,15 +260,7 @@ String normalizeNotePath(String notePath) throws IOException { * @throws IOException */ String normalizeFolderPath(String folderPath) throws IOException { - if (folderPath == null) { - throw new IOException("Folder path must not be null"); - } - - if (!folderPath.startsWith("/")) { - folderPath = "/" + folderPath; - } - - return folderPath; + return NotebookPathValidator.normalizePath(folderPath); } public void removeNote(String noteId, diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoPathValidationTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoPathValidationTest.java index 58ead149dfd..9080251e995 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoPathValidationTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoPathValidationTest.java @@ -110,4 +110,41 @@ 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)); + } } diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java index bf3d713444a..13df05b6f27 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java +++ b/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"); From 2f58906e53cf78553fd8aa4c64dd264a4075dccf Mon Sep 17 00:00:00 2001 From: gyowoo1113 Date: Wed, 9 Sep 2026 21:41:20 +0900 Subject: [PATCH 4/4] [ZEPPELIN-6692] Add tests for absolute folder path validation --- .../repo/NotebookRepoPathValidationTest.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoPathValidationTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoPathValidationTest.java index 9080251e995..f3184c88fcd 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoPathValidationTest.java +++ b/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; @@ -147,4 +148,28 @@ void normalizePath_rejects_double_dot(String path) { 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)); + } }