Fix HTTP code for file request (static, assets, file) (#3895)

* Fix HTTP code for file request (static, assets, file) * Fix bugs related to utils.abspath for symlink and unresolvale path * Requesting a directory from the file route now returns 403
2025-04-12 12:40:29 +08:00 · 2023-04-20 21:18:33 +02:00 · 2023-04-20 21:18:33 +02:00 · 5e9c3b0ac8
commit 5e9c3b0ac8
parent 1439b72f66
5 changed files with 82 additions and 36 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -12,6 +12,8 @@ No changes to highlight.
 - Fixed bug where all bokeh plots appeared in the same div by [@freddyaboulton](https://github.com/freddyaboulton) in [PR 3896](https://github.com/gradio-app/gradio/pull/3896)
 - Fixed image outputs to automatically take full output image height, unless explicitly set,  by [@aliabid94](https://github.com/aliabid94) in [PR 3905](https://github.com/gradio-app/gradio/pull/3905)
 - Fix issue in `gr.Gallery()` where setting height causes aspect ratio of images to collapse by [@dawoodkhan82](https://github.com/dawoodkhan82) in [PR 3830](https://github.com/gradio-app/gradio/pull/3830)
+- Fix issue where requesting for a non-existing file would trigger a 500 error by [@micky2be](https://github.com/micky2be) in `[PR 3895](https://github.com/gradio-app/gradio/pull/3895)`.
+- Fix bugs with abspath about symlinks, and unresolvable path on Windows by [@micky2be](https://github.com/micky2be) in `[PR 3895](https://github.com/gradio-app/gradio/pull/3895)`.

 ## Documentation Changes:

--- a/gradio/routes.py
+++ b/gradio/routes.py
@ -8,7 +8,6 @@ import inspect
 import json
 import mimetypes
 import os
-import posixpath
 import secrets
 import tempfile
 import traceback
@ -265,16 +264,12 @@ class App(FastAPI):
        @app.get("/static/{path:path}")
        def static_resource(path: str):
            static_file = safe_join(STATIC_PATH_LIB, path)
-            if static_file is not None:
-                return FileResponse(static_file)
-            raise HTTPException(status_code=404, detail="Static file not found")
+            return FileResponse(static_file)

        @app.get("/assets/{path:path}")
        def build_resource(path: str):
            build_file = safe_join(BUILD_PATH_LIB, path)
-            if build_file is not None:
-                return FileResponse(build_file)
-            raise HTTPException(status_code=404, detail="Build file not found")
+            return FileResponse(build_file)

        @app.get("/favicon.ico")
        async def favicon():
@ -309,21 +304,23 @@ class App(FastAPI):
                return RedirectResponse(
                    url=path_or_url, status_code=status.HTTP_302_FOUND
                )
-            abs_path = str(utils.abspath(path_or_url))
-            in_app_dir = utils.abspath(app.cwd) in utils.abspath(path_or_url).parents
-            created_by_app = abs_path in set().union(*blocks.temp_file_sets)
+            abs_path = utils.abspath(path_or_url)
+            in_app_dir = utils.abspath(app.cwd) in abs_path.parents
+            created_by_app = str(abs_path) in set().union(*blocks.temp_file_sets)
            in_file_dir = any(
                (
-                    utils.abspath(dir) in utils.abspath(path_or_url).parents
+                    utils.abspath(dir) in abs_path.parents
                    for dir in blocks.file_directories
                )
            )
-            was_uploaded = (
-                utils.abspath(app.uploaded_file_dir)
-                in utils.abspath(path_or_url).parents
-            )
+            was_uploaded = utils.abspath(app.uploaded_file_dir) in abs_path.parents

            if in_app_dir or created_by_app or in_file_dir or was_uploaded:
+                if not abs_path.exists():
+                    raise HTTPException(404, "File not found")
+                if abs_path.is_dir():
+                    raise HTTPException(403)
+
                range_val = request.headers.get("Range", "").strip()
                if range_val.startswith("bytes=") and "-" in range_val:
                    range_val = range_val[6:]
@ -341,8 +338,9 @@ class App(FastAPI):
                return FileResponse(abs_path, headers={"Accept-Ranges": "bytes"})

            else:
-                raise ValueError(
-                    f"File cannot be fetched: {path_or_url}. All files must contained within the Gradio python app working directory, or be a temp file created by the Gradio python app."
+                raise HTTPException(
+                    403,
+                    f"File cannot be fetched: {path_or_url}. All files must contained within the Gradio python app working directory, or be a temp file created by the Gradio python app.",
                )

        @app.get("/file/{path:path}", dependencies=[Depends(login_check)])
@ -592,26 +590,31 @@ class App(FastAPI):
 ########


-def safe_join(directory: str, path: str) -> str | None:
+def safe_join(directory: str, path: str) -> str:
    """Safely path to a base directory to avoid escaping the base directory.
    Borrowed from: werkzeug.security.safe_join"""
    _os_alt_seps: List[str] = list(
        sep for sep in [os.path.sep, os.path.altsep] if sep is not None and sep != "/"
    )

-    if path != "":
-        filename = posixpath.normpath(path)
-    else:
-        return directory
+    if path == "":
+        raise HTTPException(400)

+    filename = os.path.normpath(path)
+    fullpath = os.path.join(directory, filename)
    if (
        any(sep in filename for sep in _os_alt_seps)
        or os.path.isabs(filename)
        or filename == ".."
        or filename.startswith("../")
+        or os.path.isdir(fullpath)
    ):
-        return None
-    return posixpath.join(directory, filename)
+        raise HTTPException(403)
+
+    if not os.path.exists(fullpath):
+        raise HTTPException(404, "File not found")
+
+    return fullpath


 def get_types(cls_set: List[Type]):
--- a/gradio/utils.py
+++ b/gradio/utils.py
@ -935,10 +935,20 @@ def tex2svg(formula, *args):

 def abspath(path: str | Path) -> Path:
    """Returns absolute path of a str or Path path, but does not resolve symlinks."""
-    if Path(path).is_symlink():
+    path = Path(path)
+
+    if path.is_absolute():
+        return path
+
+    # recursively check if there is a symlink within the path
+    is_symlink = path.is_symlink() or any(
+        parent.is_symlink() for parent in path.parents
+    )
+
+    if is_symlink or path == path.resolve():  # in case path couldn't be resolved
        return Path.cwd() / path
    else:
-        return Path(path).resolve()
+        return path.resolve()


 def get_serializer_name(block: Block) -> str | None:
--- a/test/test_routes.py
+++ b/test/test_routes.py
@ -46,9 +46,9 @@ class TestRoutes:
    def test_static_files_served_safely(self, test_client):
        # Make sure things outside the static folder are not accessible
        response = test_client.get(r"/static/..%2findex.html")
-        assert response.status_code == 404
+        assert response.status_code == 403
        response = test_client.get(r"/static/..%2f..%2fapi_docs.html")
-        assert response.status_code == 404
+        assert response.status_code == 403

    def test_get_config_route(self, test_client):
        response = test_client.get("/config/")
@ -202,8 +202,8 @@ class TestRoutes:
        )
        client = TestClient(app)

-        with pytest.raises(ValueError):
-            file_response = client.get(f"/file={allowed_file.name}")
+        file_response = client.get(f"/file={allowed_file.name}")
+        assert file_response.status_code == 403

        app, _, _ = gr.Interface(lambda s: s.name, gr.File(), gr.File()).launch(
            prevent_thread_lock=True,
@ -271,6 +271,22 @@ class TestRoutes:
            assert client.get("/ps").is_success
            assert client.get("/py").is_success

+    def test_static_file_missing(self, test_client):
+        response = test_client.get(r"/static/not-here.js")
+        assert response.status_code == 404
+
+    def test_asset_file_missing(self, test_client):
+        response = test_client.get(r"/assets/not-here.js")
+        assert response.status_code == 404
+
+    def test_dynamic_file_missing(self, test_client):
+        response = test_client.get(r"/file=not-here.js")
+        assert response.status_code == 404
+
+    def test_dynamic_file_directory(self, test_client):
+        response = test_client.get(r"/file=gradio")
+        assert response.status_code == 403
+
    def test_mount_gradio_app_raises_error_if_event_queued_but_queue_disabled(self):
        with gr.Blocks() as demo:
            with gr.Row():
--- a/test/test_utils.py
+++ b/test/test_utils.py
@ -584,12 +584,27 @@ class TestAbspath:
        resolved_path = str(abspath("../gradio/gradio/test_data/lion.jpg"))
        assert ".." not in resolved_path

-    @mock.patch(
-        "pathlib.Path.is_symlink", return_value=True
-    )  # Have to patch since Windows doesn't allow creation of sym links without administrative privileges
-    def test_abspath_symlink(self, mock_islink):
-        resolved_path = str(abspath("../gradio/gradio/test_data/lion.jpg"))
-        assert ".." in resolved_path
+    @pytest.mark.skipif(
+        sys.platform.startswith("win"),
+        reason="Windows doesn't allow creation of sym links without administrative privileges",
+    )
+    def test_abspath_symlink_path(self):
+        os.symlink("gradio/test_data", "gradio/test_link", True)
+        resolved_path = str(abspath("../gradio/gradio/test_link/lion.jpg"))
+        os.unlink("gradio/test_link")
+        assert "test_link" in resolved_path
+
+    @pytest.mark.skipif(
+        sys.platform.startswith("win"),
+        reason="Windows doesn't allow creation of sym links without administrative privileges",
+    )
+    def test_abspath_symlink_dir(self):
+        os.symlink("gradio/test_data", "gradio/test_link", True)
+        full_path = os.path.join(os.getcwd(), "gradio/test_link/lion.jpg")
+        resolved_path = str(abspath(full_path))
+        os.unlink("gradio/test_link")
+        assert "test_link" in resolved_path
+        assert full_path == resolved_path


 class TestGetTypeHints: