This is an automated email from the ASF dual-hosted git repository.
tbonelee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new df3edd6ec7 [ZEPPELIN-5989] Resolve a local notebook dir as a file path
instead of parsing it as a URI
df3edd6ec7 is described below
commit df3edd6ec727921b5abc718ce5d50382dcd22567
Author: κΉλν <[email protected]>
AuthorDate: Mon Oct 5 14:01:30 2026 +0900
[ZEPPELIN-5989] Resolve a local notebook dir as a file path instead of
parsing it as a URI
### What is this PR for?
`VFSNotebookRepo.setNotebookDirectory()` parses the notebook dir with `new
URI()` unless it is a Windows absolute path like `C:\...`. A local path is not
a URI and can contain characters that a URI does not allow:
* On Windows the default dir is built as `./` + `\` + `notebook`, so the
server fails to start with `URISyntaxException: Illegal character in path at
index 2: ./\notebook`.
* On any OS, a dir with a space, like `/home/me/My Notebooks`, fails the
same way.
The issue suggests joining the dir with `Paths.get`, but on Windows that
gives `.\notebook`, which `new URI()` rejects too.
This PR:
* Resolves a notebook dir without a scheme as a file path, `new
File(getAbsoluteDir(path)).toURI()`, and parses only a dir with a scheme as a
URI. The branch that handled a URI without a scheme is removed, since that case
no longer reaches it.
* Keeps `rootNotebookFolder` as a decoded path for a local dir. It was the
root's URI with `file:///` replaced, so it kept `%20` for a space. `list()`
strips it from note file names that are decoded since ZEPPELIN-6202, which
would shift note paths by two characters per space, and `GitNotebookRepo` opens
the repository at `rootNotebookFolder/.git`, which would be a different
directory. `list()` now resolves the root by its URI instead of by this path. A
dir with another scheme keeps i [...]
For a local dir without such characters, both values are the same as before.
I could not run it on Windows. On other systems the same code path is
reached with a space, and Commons VFS treats a backslash in the file URI as a
separator. It would help if someone on Windows could start the server from this
branch without a configuration file, which is how the issue reproduces it.
### What type of PR is it?
Bug Fix
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-5989
### How should this be tested?
* `VFSNotebookRepoTest.testNotebookDirWithCharactersThatAUriDoesNotAllow`
uses a dir with a space and one with a backslash, saves and lists a note, and
checks its path.
* `GitNotebookRepoTest.notebookDirWithSpace` checks that `.git` is created
in the dir with a space and that a checkpoint is recorded.
Without the first change both tests fail with the `URISyntaxException`
above; without the second the path and `.git` checks fail.
Run locally: `VFSNotebookRepoTest` (8), `GitNotebookRepoTest` (12),
`NotebookRepoSyncTest` (10), `NotebookServiceTest` (13),
`NotebookServiceRaceConditionTest` (1), `NoteManagerMoveResaveRaceTest` (1),
`NotebookTest` (42). `testSchedule`, `testScheduleDisabledWithName` and
`testSchedulePoolUsage` fail locally with and without this change: they wait
five seconds for the cron run, and the interpreter process here takes about six
seconds to sync its libraries and register.
Closes #5518 from dev-donghwan/ZEPPELIN-5989.
Signed-off-by: ChanHo Lee <[email protected]>
---
.../zeppelin/notebook/repo/VFSNotebookRepo.java | 29 ++++++++++++++--------
.../notebook/repo/GitNotebookRepoTest.java | 16 ++++++++++++
.../notebook/repo/VFSNotebookRepoTest.java | 25 +++++++++++++++++++
3 files changed, 59 insertions(+), 11 deletions(-)
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 420a1c148a..8861aa5078 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
@@ -70,19 +70,16 @@ public class VFSNotebookRepo extends AbstractNotebookRepo {
URI filesystemRoot = null;
try {
LOGGER.info("Using notebookDir: {}", notebookDirPath);
- if (zConf.isWindowsPath(notebookDirPath)) {
- filesystemRoot = new File(notebookDirPath).toURI();
+ if (zConf.isWindowsPath(notebookDirPath) ||
!zConf.isPathWithScheme(notebookDirPath)) {
+ // A local path is not parsed as a URI: it can contain characters that
a URI does not
+ // allow, such as spaces or the backslashes of a Windows path like the
default ./\notebook.
+ filesystemRoot = new
File(zConf.getAbsoluteDir(notebookDirPath)).toURI();
} else {
filesystemRoot = new URI(notebookDirPath);
}
} catch (URISyntaxException e) {
throw new IOException(e);
}
-
- if (filesystemRoot.getScheme() == null) { // it is local path
- File f = new File(zConf.getAbsoluteDir(filesystemRoot.getPath()));
- filesystemRoot = f.toURI();
- }
this.fsManager = VFS.getManager();
this.rootNotebookFileObject = fsManager.resolveFile(filesystemRoot);
if (!this.rootNotebookFileObject.exists()) {
@@ -90,19 +87,29 @@ public class VFSNotebookRepo extends AbstractNotebookRepo {
LOGGER.info("Notebook dir doesn't exist: {}, creating it.",
rootNotebookFileObject.getName().getPath());
}
- // getPath() method returns a string without root directory in windows, so
we use getURI() instead
- // windows does not support paths with "file:///" prepended, so we replace
it by "/"
- this.rootNotebookFolder =
rootNotebookFileObject.getName().getURI().replace("file:///", "/");
+ this.rootNotebookFolder =
toRootNotebookFolder(rootNotebookFileObject.getName().getURI());
}
@Override
public Map<String, NoteInfo> list(AuthenticationInfo subject) throws
IOException {
// Must to create rootNotebookFileObject each time when call method list,
otherwise we can not
// get the updated data under this folder.
- this.rootNotebookFileObject =
fsManager.resolveFile(this.rootNotebookFolder);
+ this.rootNotebookFileObject =
fsManager.resolveFile(rootNotebookFileObject.getName().getURI());
return listFolder(rootNotebookFileObject);
}
+ /**
+ * A local root is kept as a decoded path, like the note file names in
{@link #listFolder}, so
+ * that {@link #getNotePath} can strip it from them and GitNotebookRepo can
open the repository
+ * in it. Windows does not support paths with "file:///" prepended, so it
starts with "/".
+ */
+ private static String toRootNotebookFolder(String rootUri) {
+ if (rootUri.startsWith("file:")) {
+ return URI.create(rootUri).getPath();
+ }
+ return rootUri;
+ }
+
private Map<String, NoteInfo> listFolder(FileObject fileObject) throws
IOException {
Map<String, NoteInfo> noteInfos = new HashMap<>();
diff --git
a/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java
b/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java
index 3996ed2b03..78a2bc6d6e 100644
---
a/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java
+++
b/zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/GitNotebookRepoTest.java
@@ -96,6 +96,22 @@ class GitNotebookRepoTest {
FileUtils.deleteDirectory(zeppelinDir);
}
+ @Test
+ void notebookDirWithSpace() throws IOException {
+ File dirWithSpace = new File(zeppelinDir, "my notebooks");
+ FileUtils.moveDirectory(notebooksDir, dirWithSpace);
+ zConf.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_DIR.getVarName(),
dirWithSpace.getAbsolutePath());
+
+ notebookRepo = new GitNotebookRepo();
+ notebookRepo.init(zConf, noteParser);
+
+ // The repository is opened in the notebook dir, not in a sibling named
after its URI form.
+ assertTrue(new File(dirWithSpace, ".git").isDirectory());
+ assertEquals(TEST_NOTE_PATH,
notebookRepo.list(null).get(TEST_NOTE_ID).getPath());
+ notebookRepo.checkpoint(TEST_NOTE_ID, TEST_NOTE_PATH, "first commit",
null);
+ assertEquals(1, notebookRepo.revisionHistory(TEST_NOTE_ID, TEST_NOTE_PATH,
null).size());
+ }
+
@Test
void initNonemptyNotebookDir() throws IOException, GitAPIException {
//given - .git does not exit
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 c7dcc9bdc4..fb8d7d2527 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
@@ -30,16 +30,19 @@ import org.apache.zeppelin.user.AuthenticationInfo;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
import java.io.File;
import java.nio.file.Files;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
import java.util.List;
import java.util.Map;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
class VFSNotebookRepoTest {
@@ -115,6 +118,28 @@ class VFSNotebookRepoTest {
assertEquals(1, notebookRepo.list(AuthenticationInfo.ANONYMOUS).size());
}
+ @Test
+ void testNotebookDirWithCharactersThatAUriDoesNotAllow(@TempDir Path
tempDir) throws IOException {
+ // A space anywhere, or the backslashes of a Windows path such as the
default "./\\notebook".
+ // Commons VFS treats a backslash as a separator, so only check that the
repo works.
+ for (String dirName : new String[] {"my notebooks", "back\\slash"}) {
+
zConf.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_NOTEBOOK_DIR.getVarName(),
+ tempDir.resolve(dirName).toString());
+ VFSNotebookRepo repo = new VFSNotebookRepo();
+ repo.init(zConf, noteParser);
+
+ assertTrue(new File(repo.rootNotebookFolder).isDirectory(), dirName);
+ Note note = new Note();
+ note.setPath("/note1");
+ note.setNoteParser(noteParser);
+ repo.save(note, AuthenticationInfo.ANONYMOUS);
+ Map<String, NoteInfo> noteInfos =
repo.list(AuthenticationInfo.ANONYMOUS);
+ assertEquals(1, noteInfos.size(), dirName);
+ assertEquals("/note1", noteInfos.get(note.getId()).getPath(), dirName);
+ }
+ assertTrue(tempDir.resolve("my notebooks").toFile().isDirectory());
+ }
+
@Test
void testNoteNameWithColon() throws IOException {
assertEquals(0, notebookRepo.list(AuthenticationInfo.ANONYMOUS).size());
