Skip to content

Conversation

@ParkGyeongTae
Copy link
Member

@ParkGyeongTae ParkGyeongTae commented Jun 15, 2025

What is this PR for?

This PR fixes a UI bug where notebook and folder names that contain spaces are displayed with the URL-encoded text %20 in the notebook tree (e.g., Flink%20Tutorial).
We now URL-decode the VFS path and keep the existing Windows-specific prefix handling, so names with spaces render correctly across all platforms.

What type of PR is it?

Bug Fix

Todos

  • - Add URL-decode logic in path normalisation

What is the Jira issue?

How should this be tested?

  • Build this branch and start Zeppelin.
  • Create or import a notebook whose name contains a space (e.g., “Python Tutorial”).

Screenshots (if appropriate)

Before
image
After
image

Questions:

  • Does the license files need to update? No.
  • Is there breaking changes for older versions? No.
  • Does this needs documentation? No.

Copy link
Contributor

@Reamer Reamer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the wrong place. Please execute the URLDecode closer to the request/response code, maybe also in the websocket part
Background: There are more NotebookRepos then VFSNotebookRepo.

@tbonelee
Copy link
Contributor

tbonelee commented Jun 28, 2025

@Reamer

Hi Reamer, thanks for the review.

Just a thought, the need for decoding here might stem from the use of getURI() instead of getPath(), which returns an encoded string. Since encoding alters the original path, decoding it at this point could help restore its intended form. I also noticed that S3NotebookRepo provides decoded names, so this seemed consistent-but I haven't checked all other repos, so I may be missing some cases.

Curious to hear your thoughts.

@Reamer
Copy link
Contributor

Reamer commented Jun 30, 2025

Javadoc of getPath().

    /**
     * Gets the absolute path string of this file, within its file system. This path is normalized, so that {@code .}
     * and {@code ..} elements have been removed. Also, the path only contains {@code /} as its separator character. The
     * path always starts with {@code /}
     * <p>
     * The root of a file system has {@code /} as its absolute path.
     * </p>
     *
     * @return The path. Never returns null.
     */
    String getPath();

Maybe it was a bug in vfs library. Do you have access to a Windows machine to validate the behaviour of the workaround with getUri()?

I would like to get rid of the workaround.

@Reamer
Copy link
Contributor

Reamer commented Jun 30, 2025

I also noticed that S3NotebookRepo provides decoded names, so this seemed consistent-but I haven't checked all other repos, so I may be missing some cases.

I thought the method calls go to a Java native object. But here the vfs library is used. Therefore it must be corrected at this point.

@ParkGyeongTae
Copy link
Member Author

@Reamer
I discovered something new. When Zeppelin is running, notebook names with spaces are displayed correctly. However, after shutting down and restarting Zeppelin, the spaces in notebook names are shown as %20.

Interestingly, if I create a new notebook before restarting, the space is displayed properly. But after restarting, even that notebook shows %20 instead of spaces.

To fix this, I modified the NoteInfo.java file located at zeppelin/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteInfo.java.

What do you think about this approach?

Initial launch
image

Notebook created while running
image

After restart
image

@Reamer
Copy link
Contributor

Reamer commented Jul 4, 2025

Very strange. Which NotebookRepo and which Zeppelin version are you using? I have never seen %20 in the name of Zeppelin Notes. I have never seen %20 in the name of Zeppelin Notes.

@ParkGyeongTae
Copy link
Member Author

@Reamer
I checked out the master branch and ran it after building in IntelliJ. I'll look into this issue further.

@tbonelee
Copy link
Contributor

tbonelee commented Jul 6, 2025

@ParkGyeongTae After checking with the dubugger, I found that the reason why the path of a newly created note after startup appears correctly is because the Note instance is not initialized using the path from the .getURI() method. Instead, the path from the web app(from the user) is used to initialize the Note instance, and this path remains unchanged when returned to the web app. So I believe we just need to fix the previously claimed bugs.

@ParkGyeongTae
Copy link
Member Author

@tbonelee
Ah, I see. That makes sense now—thank you for the clear explanation!
I'll revert back to the previous approach where I modified VFSNotebookRepo.java.
By the way, are you experiencing the same issue as I did?

@tbonelee
Copy link
Contributor

tbonelee commented Jul 7, 2025

Yes, I'm seeing the same. The paths are shown in the URI encoded format as well.

@ParkGyeongTae
Copy link
Member Author

@Reamer
cc. @tbonelee
I’ve tested this on both an Intel MacBook and an M4 MacBook, and the same issue occurs on both. After cloning Zeppelin from GitHub, building it with the default settings, and running it, spaces in note names are shown as %20.

@Reamer
Copy link
Contributor

Reamer commented Jul 21, 2025

@ParkGyeongTae Sorry, for the late response.
Can you try the getPath() method on your platform so maybe we can get rid of all the getUri() decode encode stuff? See:
#4947 (comment)

@ParkGyeongTae
Copy link
Member Author

@Reamer
I tested it by replacing getURI() with getPath(), and everything works fine on my platform.

@Reamer
Copy link
Contributor

Reamer commented Jul 21, 2025

@Reamer I tested it by replacing getURI() with getPath(), and everything works fine on my platform.

Which platform did you use?

@ParkGyeongTae
Copy link
Member Author

@Reamer
I'm using a MacBook M4 Pro with macOS.
Do you want to check how it behaves when built on Windows?

@Reamer
Copy link
Contributor

Reamer commented Jul 24, 2025

It's good to hear that it works on MacOS. I would be interested to know whether it also works on Windows. As far as I know, Windows itself is not supported by Zeppelin, but we shouldn't break any functions.

@tbonelee
Copy link
Contributor

tbonelee commented Jul 24, 2025

I checked this on my old Windows laptop, and the latest revision breaks the getNotePath() method because rootNotebookFolder doesn't align with noteFileName.

  • On Windows:
    • rootNotebookFolder : /C:/Users/chanlee/...
    • noteFileName: /Users/chanlee/...
    • => The prefixes don't match.

If we try to fix this by setting rootNotebookFolder using getName().getPath() also, the /C: segment gets dropped, which breaks file access.
(This effectively reverts the fix from #3615 .)

To fix the URL-encoded path issue without breaking things on Windows, we should ensure:

  • rootNotebookFolder includes the drive letter (e.g., C:) at the beginning.
  • noteFileName starts with the same prefix as rootNotebookFolder. (If the former includes the drive letter, the latter should as well)

@ParkGyeongTae
Copy link
Member Author

@tbonelee
cc. @Reamer
Hi Chanho, Thanks a lot for checking this on Windows!

It makes sense now that directly replacing getURI() with getPath() wouldn't work due to the mismatch between rootNotebookFolder and noteFileName on Windows. That’s a really helpful insight.

Given that, what do you think about keeping getURI() but just applying decoding instead of replacing it with getPath()?

Original code:

String noteFileName = fileObject.getName().getURI().replace("file:///", "/");

Proposed changed:

String noteFileName = URLDecoder.decode(
    fileObject.getName().getURI(), StandardCharsets.UTF_8
).replace("file:///", "/");

This might fix the encoded path issue without affecting the drive letter or breaking things on Windows. Would love to hear your thoughts!

@Reamer
Copy link
Contributor

Reamer commented Jul 25, 2025

Thank you for recreating the behavior on Windows. Making a major code change here would be going too far. We can therefore take the workaround via the URL decoder.

Copy link
Contributor

@Reamer Reamer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If no further comments are received, I will merge the pull request into master and branch-0.12.

@Reamer Reamer merged commit 890ddfe into apache:master Jul 29, 2025
16 of 18 checks passed
asf-gitbox-commits pushed a commit that referenced this pull request Jul 29, 2025
… file paths

### What is this PR for?
This PR fixes a UI bug where notebook and folder names that contain spaces are displayed with the URL-encoded text %20 in the notebook tree (e.g., Flink%20Tutorial).
We now URL-decode the VFS path and keep the existing Windows-specific prefix handling, so names with spaces render correctly across all platforms.

### What type of PR is it?
Bug Fix

### Todos
* [x] - Add URL-decode logic in path normalisation

### What is the Jira issue?
* Jira: https://issues.apache.org/jira/browse/ZEPPELIN/ZEPPELIN-6202

### How should this be tested?
* Build this branch and start Zeppelin.
* Create or import a notebook whose name contains a space (e.g., “Python Tutorial”).

### Screenshots (if appropriate)
Before
![image](https://github.com/user-attachments/assets/f1fba587-fd88-4034-b224-f2edc08e5f1d)
After
![image](https://github.com/user-attachments/assets/0577c8c2-6f48-475f-b47e-30afd144bfd3)

### Questions:
* Does the license files need to update? No.
* Is there breaking changes for older versions? No.
* Does this needs documentation? No.

Closes #4947 from ParkGyeongTae/ZEPPELIN-6202.

Signed-off-by: Philipp Dallig <philipp.dallig@gmail.com>
(cherry picked from commit 890ddfe)
Signed-off-by: Philipp Dallig <philipp.dallig@gmail.com>
@Reamer
Copy link
Contributor

Reamer commented Jul 29, 2025

Merged into master and branch-0.12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants