Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize os_path #886

Merged
merged 1 commit into from
Jun 22, 2022
Merged

Conversation

martinRenou
Copy link
Contributor

@martinRenou martinRenou commented Jun 22, 2022

This fixes an issue causing access to local files in Voila considered forbidden:

Accessing a file ./jupyter.svg in Voila (jupyter.svg being under the root_dir) results in a:

  File "/home/martin/micromamba/envs/voila/lib/python3.10/site-packages/mistune.py", line 661, in _process_link
    return self.renderer.image(link, title, text)
  File "/home/martin/github/voila/voila/exporter.py", line 34, in image
    content = contents_manager.get(src, format='base64')
  File "/home/martin/micromamba/envs/voila/lib/python3.10/site-packages/jupyter_server/services/contents/filemanager.py", line 386, in get
    raise web.HTTPError(404, four_o_four)
tornado.web.HTTPError: HTTP 404: Not Found (file or directory does not exist: './jupyter.svg')

This was because the root_dir was set to /home/martin/github/voila/tests/notebooks and the path /home/martin/github/voila/tests/notebooks/./jupyter.svg was considered "hidden" by jupyter_server, while it's not. Normalizing the path fixes it.

@davidbrochart
Copy link
Contributor

davidbrochart commented Jun 22, 2022

This also transforms the path to an absolute path, which we may not want.

@davidbrochart
Copy link
Contributor

My bad, it doesn't.

@martinRenou
Copy link
Contributor Author

Oh cool! Thanks for checking

@martinRenou
Copy link
Contributor Author

I don't have the rights for adding labels. Maybe a maintenance or bug label would fit?

@martinRenou
Copy link
Contributor Author

It would be amazing if this could be backported to a 1.17.2 🙏🏽

Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thanks!

@blink1073 blink1073 merged commit dee8c7a into jupyter-server:main Jun 22, 2022
@blink1073
Copy link
Contributor

@meeseeksdev please backport to 1.x

meeseeksmachine pushed a commit to meeseeksmachine/jupyter_server that referenced this pull request Jun 22, 2022
blink1073 pushed a commit that referenced this pull request Jun 22, 2022
@martinRenou martinRenou deleted the normalize_os_path branch June 23, 2022 06:59
@martinRenou
Copy link
Contributor Author

Thank you!

@krassowski
Copy link
Collaborator

Bumping urgency of a patch release soon: the issue also affects jupyterlab-git (Error Loading Notebook Diff: file or directory does not exist: './notebook.ipynb'). I verified that this PR fixes it there too. Thank you all for the quick fix and backport!

@blink1073
Copy link
Contributor

In progress now

krassowski added a commit to krassowski/notebook that referenced this pull request Sep 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants