From 252e8a2cdace406efd87c78ec91b3317114d7cb0 Mon Sep 17 00:00:00 2001 From: sambloomquist Date: Tue, 7 Nov 2023 20:33:02 -0600 Subject: [PATCH 1/4] contents handler 404 rather than raise exc --- jupyter_server/services/contents/handlers.py | 31 +++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/jupyter_server/services/contents/handlers.py b/jupyter_server/services/contents/handlers.py index 15b3a5c920..93db97fcbd 100644 --- a/jupyter_server/services/contents/handlers.py +++ b/jupyter_server/services/contents/handlers.py @@ -5,6 +5,7 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. import json +from http import HTTPStatus try: from jupyter_client.jsonutil import json_default @@ -116,18 +117,26 @@ async def get(self, path=""): content = int(content_str or "") if not cm.allow_hidden and await ensure_async(cm.is_hidden(path)): - raise web.HTTPError(404, f"file or directory {path!r} does not exist") - - model = await ensure_async( - self.contents_manager.get( - path=path, - type=type, - format=format, - content=content, + self.set_status(HTTPStatus.NOT_FOUND) + self.write(f"file or directory {path!r} does not exist") + await self.finish() + try: + model = await ensure_async( + self.contents_manager.get( + path=path, + type=type, + format=format, + content=content, + ) ) - ) - validate_model(model, expect_content=content) - self._finish_model(model, location=False) + validate_model(model, expect_content=content) + self._finish_model(model, location=False) + except web.HTTPError as exc: + if exc.status_code == HTTPStatus.NOT_FOUND: + self.set_status(HTTPStatus.NOT_FOUND) + self.write(f"file or directory {path!r} does not exist") + await self.finish() + raise @web.authenticated @authorized From 3d5cb0a706b0fce89a7bb9b1f0d2ce0439e0c1c0 Mon Sep 17 00:00:00 2001 From: sambloomquist Date: Thu, 9 Nov 2023 20:08:02 -0600 Subject: [PATCH 2/4] test case for file not found --- tests/services/contents/test_manager.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/services/contents/test_manager.py b/tests/services/contents/test_manager.py index 7fa5cbd742..6f51952cfb 100644 --- a/tests/services/contents/test_manager.py +++ b/tests/services/contents/test_manager.py @@ -436,6 +436,15 @@ async def test_404(jp_file_contents_manager_class, tmp_path): except HTTPError as e: assert e.status_code == 404 + # Test file not found + td = str(tmp_path) + cm = jp_file_contents_manager_class(root_dir=td) + os_path = cm._get_os_path(td) + not_a_file = os.path.join(os_path, "foo.bar") + with pytest.raises(HTTPError) as excinfo: + result = await ensure_async(cm.get(not_a_file, "w")) + assert excinfo.value.status_code == 404 + async def test_escape_root(jp_file_contents_manager_class, tmp_path): td = str(tmp_path) From 33f55b55ebb354dbe70b2d27df555f01d9dc4f84 Mon Sep 17 00:00:00 2001 From: sambloomquist Date: Sun, 12 Nov 2023 19:06:29 -0600 Subject: [PATCH 3/4] fix `test_manager::test_404` logic --- tests/services/contents/test_manager.py | 54 ++++++++++++------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/tests/services/contents/test_manager.py b/tests/services/contents/test_manager.py index 6f51952cfb..8d4052dd2d 100644 --- a/tests/services/contents/test_manager.py +++ b/tests/services/contents/test_manager.py @@ -406,43 +406,43 @@ async def test_400(jp_file_contents_manager_class, tmp_path): # noqa async def test_404(jp_file_contents_manager_class, tmp_path): + # setup + td = str(tmp_path) + cm = jp_file_contents_manager_class(root_dir=td) + # Test visible file in hidden folder - with pytest.raises(HTTPError) as excinfo: - td = str(tmp_path) - cm = jp_file_contents_manager_class(root_dir=td) - hidden_dir = ".hidden" - file_in_hidden_path = os.path.join(hidden_dir, "visible.txt") - _make_dir(cm, hidden_dir) - model = await ensure_async(cm.new(path=file_in_hidden_path)) - os_path = cm._get_os_path(model["path"]) + cm.allow_hidden = True + hidden_dir = ".hidden" + file_in_hidden_path = os.path.join(hidden_dir, "visible.txt") + _make_dir(cm, hidden_dir) + model = await ensure_async(cm.new(path=file_in_hidden_path)) + os_path = cm._get_os_path(model["path"]) + cm.allow_hidden = False - try: - result = await ensure_async(cm.get(os_path, "w")) - except HTTPError as e: - assert e.status_code == 404 + with pytest.raises(HTTPError) as excinfo: + await ensure_async(cm.get(os_path)) + assert excinfo.value.status_code == 404 # Test hidden file in visible folder - with pytest.raises(HTTPError) as excinfo: - td = str(tmp_path) - cm = jp_file_contents_manager_class(root_dir=td) - hidden_dir = "visible" - file_in_hidden_path = os.path.join(hidden_dir, ".hidden.txt") - _make_dir(cm, hidden_dir) - model = await ensure_async(cm.new(path=file_in_hidden_path)) - os_path = cm._get_os_path(model["path"]) + cm.allow_hidden = True + hidden_dir = "visible" + file_in_hidden_path = os.path.join(hidden_dir, ".hidden.txt") + _make_dir(cm, hidden_dir) + model = await ensure_async(cm.new(path=file_in_hidden_path)) + os_path = cm._get_os_path(model["path"]) + cm.allow_hidden = False - try: - result = await ensure_async(cm.get(os_path, "w")) - except HTTPError as e: - assert e.status_code == 404 + with pytest.raises(HTTPError) as excinfo: + await ensure_async(cm.get(os_path)) + assert excinfo.value.status_code == 404 # Test file not found td = str(tmp_path) cm = jp_file_contents_manager_class(root_dir=td) - os_path = cm._get_os_path(td) - not_a_file = os.path.join(os_path, "foo.bar") + not_a_file = "foo.bar" + with pytest.raises(HTTPError) as excinfo: - result = await ensure_async(cm.get(not_a_file, "w")) + await ensure_async(cm.get(not_a_file)) assert excinfo.value.status_code == 404 From dedb3932f75443bb81e8e449391702bc0ca9f817 Mon Sep 17 00:00:00 2001 From: sambloomquist Date: Tue, 14 Nov 2023 19:56:46 -0600 Subject: [PATCH 4/4] add _finish_error method and code comment --- jupyter_server/services/contents/handlers.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/jupyter_server/services/contents/handlers.py b/jupyter_server/services/contents/handlers.py index 93db97fcbd..4a3dbab19f 100644 --- a/jupyter_server/services/contents/handlers.py +++ b/jupyter_server/services/contents/handlers.py @@ -92,6 +92,12 @@ def _finish_model(self, model, location=True): self.set_header("Content-Type", "application/json") self.finish(json.dumps(model, default=json_default)) + async def _finish_error(self, code, message): + """Finish a JSON request with an error code and descriptive message""" + self.set_status(code) + self.write(message) + await self.finish() + @web.authenticated @authorized async def get(self, path=""): @@ -117,9 +123,9 @@ async def get(self, path=""): content = int(content_str or "") if not cm.allow_hidden and await ensure_async(cm.is_hidden(path)): - self.set_status(HTTPStatus.NOT_FOUND) - self.write(f"file or directory {path!r} does not exist") - await self.finish() + await self._finish_error( + HTTPStatus.NOT_FOUND, f"file or directory {path!r} does not exist" + ) try: model = await ensure_async( self.contents_manager.get( @@ -132,10 +138,11 @@ async def get(self, path=""): validate_model(model, expect_content=content) self._finish_model(model, location=False) except web.HTTPError as exc: + # 404 is okay in this context, catch exception and return 404 code to prevent stack trace on client if exc.status_code == HTTPStatus.NOT_FOUND: - self.set_status(HTTPStatus.NOT_FOUND) - self.write(f"file or directory {path!r} does not exist") - await self.finish() + await self._finish_error( + HTTPStatus.NOT_FOUND, f"file or directory {path!r} does not exist" + ) raise @web.authenticated