Skip to content

Commit 3b34f07

Browse files
committed
[ZEPPELIN-6692] Extract shared path normalization for note and folder paths
1 parent a222a66 commit 3b34f07

4 files changed

Lines changed: 68 additions & 19 deletions

File tree

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,31 @@ public static String decodeRepeatedly(String encoded) throws IOException {
8585
throw new IOException("Exceeded maximum decode attempts. Possible malicious input.");
8686
}
8787

88+
/**
89+
* Normalizes a path using the rules shared by note and folder paths.
90+
*
91+
* @param path the path to normalize
92+
* @return the normalized path
93+
* @throws IOException if the path cannot be normalized
94+
*/
95+
public static String normalizePath(String path) throws IOException {
96+
if (path == null) {
97+
throw new IOException("Path must not be null");
98+
}
99+
100+
if (!path.startsWith("/")) {
101+
path = "/" + path;
102+
}
103+
104+
path = decodeRepeatedly(path);
105+
106+
if (path.contains("..")) {
107+
throw new IOException("Path can not contain '..'");
108+
}
109+
110+
return path;
111+
}
112+
88113
/**
89114
* Requires {@code folderPath} to use the canonical absolute folder-path form.
90115
*

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

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -234,13 +234,11 @@ String normalizeNotePath(String notePath) throws IOException {
234234
if (StringUtils.isBlank(notePath)) {
235235
notePath = "/Untitled Note";
236236
}
237-
if (!notePath.startsWith("/")) {
238-
notePath = "/" + notePath;
239-
}
240237

241238
notePath = notePath.replace("\r", " ").replace("\n", " ");
242239

243-
notePath = NotebookPathValidator.decodeRepeatedly(notePath);
240+
notePath = NotebookPathValidator.normalizePath(notePath);
241+
244242
if (notePath.endsWith("/")) {
245243
throw new IOException("Note name shouldn't end with '/'");
246244
}
@@ -250,9 +248,6 @@ String normalizeNotePath(String notePath) throws IOException {
250248
throw new IOException("Note name must be less than 255");
251249
}
252250

253-
if (notePath.contains("..")) {
254-
throw new IOException("Note name can not contain '..'");
255-
}
256251
return notePath;
257252
}
258253

@@ -265,15 +260,7 @@ String normalizeNotePath(String notePath) throws IOException {
265260
* @throws IOException
266261
*/
267262
String normalizeFolderPath(String folderPath) throws IOException {
268-
if (folderPath == null) {
269-
throw new IOException("Folder path must not be null");
270-
}
271-
272-
if (!folderPath.startsWith("/")) {
273-
folderPath = "/" + folderPath;
274-
}
275-
276-
return folderPath;
263+
return NotebookPathValidator.normalizePath(folderPath);
277264
}
278265

279266
public void removeNote(String noteId,

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,4 +110,41 @@ void decodeRepeatedly_accepts_max_decode_layers() throws IOException {
110110
// cleanly; the constant means *layers*, not raw loop iterations.
111111
assertEquals("/..", NotebookPathValidator.decodeRepeatedly("/%252525252e%252525252e"));
112112
}
113+
114+
@Test
115+
void normalizePath_adds_leading_slash() throws IOException {
116+
assertEquals("/folder/note", NotebookPathValidator.normalizePath("folder/note"));
117+
}
118+
119+
@Test
120+
void normalizePath_keeps_existing_leading_slash() throws IOException {
121+
assertEquals("/folder/note", NotebookPathValidator.normalizePath("/folder/note"));
122+
}
123+
124+
@Test
125+
void normalizePath_decodes_url_encoding() throws IOException {
126+
assertEquals("/folder/My Note", NotebookPathValidator.normalizePath("/folder/My%20Note"));
127+
}
128+
129+
@Test
130+
void normalizePath_decodes_repeated_url_encoding() throws IOException {
131+
assertEquals("/folder/My Note", NotebookPathValidator.normalizePath("/folder/My%2520Note"));
132+
}
133+
134+
@ParameterizedTest
135+
@ValueSource(strings = {
136+
"/foo/../bar",
137+
"/foo..bar",
138+
"/...",
139+
"/%2e%2e/bar",
140+
"/%252e%252e/bar"
141+
})
142+
void normalizePath_rejects_double_dot(String path) {
143+
assertThrows(IOException.class, () -> NotebookPathValidator.normalizePath(path));
144+
}
145+
146+
@Test
147+
void normalizePath_rejects_null() {
148+
assertThrows(IOException.class, () -> NotebookPathValidator.normalizePath(null));
149+
}
113150
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -796,20 +796,20 @@ void testNormalizeNotePath() throws IOException {
796796
notebookService.normalizeNotePath("my..note");
797797
fail("Should fail");
798798
} catch (IOException e) {
799-
assertEquals("Note name can not contain '..'", e.getMessage());
799+
assertEquals("Path can not contain '..'", e.getMessage());
800800
}
801801
try {
802802
notebookService.normalizeNotePath("%2e%2e/%2e%2e/tmp/test222");
803803
fail("Should fail");
804804
} catch (IOException e) {
805-
assertEquals("Note name can not contain '..'", e.getMessage());
805+
assertEquals("Path can not contain '..'", e.getMessage());
806806
}
807807
try {
808808
// Double URL encoding of ".."
809809
notebookService.normalizeNotePath("%252e%252e/%252e%252e/tmp/test333");
810810
fail("Should fail");
811811
} catch (IOException e) {
812-
assertEquals("Note name can not contain '..'", e.getMessage());
812+
assertEquals("Path can not contain '..'", e.getMessage());
813813
}
814814
try {
815815
notebookService.normalizeNotePath("%25252525252e%25252525252e/tmp/test444");

0 commit comments

Comments
 (0)