Skip to content

Commit

Permalink
Do not attach content_disposition_type = "attachment" headers for f…
Browse files Browse the repository at this point in the history
…iles explicitly allowed by developer (#9348)

* changes

* add changeset

* format

* fix type

* type

* add test

---------

Co-authored-by: gradio-pr-bot <gradio-pr-bot@users.noreply.github.com>
  • Loading branch information
abidlabs and gradio-pr-bot authored Sep 13, 2024
1 parent 5b86e2f commit 61f794b
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 31 deletions.
5 changes: 5 additions & 0 deletions .changeset/young-candles-stare.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"gradio": minor
---

feat:Do not attach `content_disposition_type = "attachment"` headers for files explicitly allowed by developer
6 changes: 2 additions & 4 deletions gradio/blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,6 @@


if TYPE_CHECKING: # Only import for type checking (is False at runtime).
from fastapi.applications import FastAPI

from gradio.components.base import Component
from gradio.renderable import Renderable

Expand Down Expand Up @@ -2227,7 +2225,7 @@ def launch(
_frontend: bool = True,
enable_monitoring: bool | None = None,
strict_cors: bool = True,
) -> tuple[FastAPI, str, str]:
) -> tuple[routes.App, str, str]:
"""
Launches a simple web server that serves the demo. Can also be used to create a
public link used by anyone to access the demo from their browser by setting share=True.
Expand All @@ -2253,7 +2251,7 @@ def launch(
ssl_verify: If False, skips certificate validation which allows self-signed certificates to be used.
quiet: If True, suppresses most print statements.
show_api: If True, shows the api docs in the footer of the app. Default True.
allowed_paths: List of complete filepaths or parent directories that gradio is allowed to serve. Must be absolute paths. Warning: if you provide directories, any files in these directories or their subdirectories are accessible to all users of your app. Can be set by comma separated environment variable GRADIO_ALLOWED_PATHS.
allowed_paths: List of complete filepaths or parent directories that gradio is allowed to serve. Must be absolute paths. Warning: if you provide directories, any files in these directories or their subdirectories are accessible to all users of your app. Can be set by comma separated environment variable GRADIO_ALLOWED_PATHS. These files are generally assumed to be secure and will be displayed in the browser when possible.
blocked_paths: List of complete filepaths or parent directories that gradio is not allowed to serve (i.e. users of your app are not allowed to access). Must be absolute paths. Warning: takes precedence over `allowed_paths` and all other directories exposed by Gradio by default. Can be set by comma separated environment variable GRADIO_BLOCKED_PATHS.
root_path: The root path (or "mount point") of the application, if it's not served from the root ("/") of the domain. Often used when the application is behind a reverse proxy that forwards requests to the application. For example, if the application is served at "https://example.com/myapp", the `root_path` should be set to "/myapp". A full URL beginning with http:// or https:// can be provided, which will be used as the root path in its entirety. Can be set by environment variable GRADIO_ROOT_PATH. Defaults to "".
app_kwargs: Additional keyword arguments to pass to the underlying FastAPI app as a dictionary of parameter keys and argument values. For example, `{"docs_url": "/docs"}`
Expand Down
18 changes: 9 additions & 9 deletions gradio/processing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -512,18 +512,18 @@ def _check_allowed(path: str | Path, check_in_upload_folder: bool):

abs_path = utils.abspath(path)

# if check_in_upload_folder=True
# we are running this during pre-process
# in which case only files in the upload_folder (cache_dir)
# are accepted
allowed = [utils.get_upload_folder()]
if not check_in_upload_folder:
allowed += blocks.allowed_paths + [os.getcwd(), tempfile.gettempdir()]

created_paths = [utils.get_upload_folder()]
# if check_in_upload_folder=True, we are running this during pre-process
# in which case only files in the upload_folder (cache_dir) are accepted
if check_in_upload_folder:
allowed_paths = []
else:
allowed_paths = blocks.allowed_paths + [os.getcwd(), tempfile.gettempdir()]
allowed, reason = utils.is_allowed_file(
abs_path,
blocked_paths=blocks.blocked_paths,
allowed_paths=allowed,
allowed_paths=allowed_paths,
created_paths=created_paths,
)
if not allowed:
msg = f"Cannot move {abs_path} to the gradio cache dir because "
Expand Down
11 changes: 5 additions & 6 deletions gradio/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -574,19 +574,18 @@ async def file(path_or_url: str, request: fastapi.Request):

from gradio.data_classes import _StaticFiles

allowed, _ = utils.is_allowed_file(
allowed, reason = utils.is_allowed_file(
abs_path,
blocked_paths=blocks.blocked_paths,
allowed_paths=blocks.allowed_paths
+ [app.uploaded_file_dir, utils.get_cache_folder()]
+ _StaticFiles.all_paths,
allowed_paths=blocks.allowed_paths + _StaticFiles.all_paths,
created_paths=[app.uploaded_file_dir, utils.get_cache_folder()],
)
if not allowed:
raise HTTPException(403, f"File not allowed: {path_or_url}.")

mime_type, _ = mimetypes.guess_type(abs_path)
if mime_type in XSS_SAFE_MIMETYPES:
media_type = mime_type
if mime_type in XSS_SAFE_MIMETYPES or reason == "allowed":
media_type = mime_type or "application/octet-stream"
content_disposition_type = "inline"
else:
media_type = "application/octet-stream"
Expand Down
13 changes: 7 additions & 6 deletions gradio/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1468,16 +1468,17 @@ def is_allowed_file(
path: Path,
blocked_paths: Sequence[str | Path],
allowed_paths: Sequence[str | Path],
) -> tuple[bool, Literal["in_blocklist", "allowed", "not_created_or_allowed"]]:
created_paths: Sequence[str | Path],
) -> tuple[
bool, Literal["in_blocklist", "allowed", "created", "not_created_or_allowed"]
]:
in_blocklist = any(
is_in_or_equal(path, blocked_path) for blocked_path in blocked_paths
)
if in_blocklist:
return False, "in_blocklist"

in_allowedlist = any(
is_in_or_equal(path, allowed_path) for allowed_path in allowed_paths
)
if in_allowedlist:
if any(is_in_or_equal(path, allowed_path) for allowed_path in allowed_paths):
return True, "allowed"
if any(is_in_or_equal(path, created_path) for created_path in created_paths):
return True, "created"
return False, "not_created_or_allowed"
16 changes: 14 additions & 2 deletions test/test_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,17 +273,29 @@ def test_response_attachment_format(self):
app, _, _ = io.launch(
prevent_thread_lock=True,
allowed_paths=[
os.path.dirname(image_file.name),
os.path.dirname(html_file.name),
image_file.name,
html_file.name,
],
)

html_file2 = tempfile.NamedTemporaryFile(
mode="w", delete=False, suffix=".html", dir=app.uploaded_file_dir
)
html_file2.write("<html>Hello, world!</html>")
html_file2.flush()
html_file2_name = str(Path(app.uploaded_file_dir) / html_file2.name)

client = TestClient(app)

file_response = client.get(f"{API_PREFIX}/file={image_file.name}")
assert file_response.headers["Content-Type"] == "image/png"
assert "inline" in file_response.headers["Content-Disposition"]

file_response = client.get(f"{API_PREFIX}/file={html_file.name}")
assert file_response.headers["Content-Type"] == "text/html; charset=utf-8"
assert "inline" in file_response.headers["Content-Disposition"]

file_response = client.get(f"{API_PREFIX}/file={html_file2_name}")
assert file_response.headers["Content-Type"] == "application/octet-stream"
assert "attachment" in file_response.headers["Content-Disposition"]

Expand Down
12 changes: 8 additions & 4 deletions test/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -429,24 +429,26 @@ def test_is_in_or_equal_fuzzer(path_1, path_2):
path=create_path_string(),
blocked_paths=create_path_list(),
allowed_paths=create_path_list(),
created_paths=create_path_list(),
)
def test_is_allowed_file_fuzzer(
path: Path,
blocked_paths: Sequence[Path],
allowed_paths: Sequence[Path],
created_paths: Sequence[Path],
):
result, reason = is_allowed_file(path, blocked_paths, allowed_paths)
result, reason = is_allowed_file(path, blocked_paths, allowed_paths, created_paths)

assert isinstance(result, bool)
assert reason in [
"in_blocklist",
"allowed",
"not_created_or_allowed",
"created_by_app",
"created",
]

if result:
assert reason == "allowed"
assert reason in ("allowed", "created")
elif reason == "in_blocklist":
assert any(is_in_or_equal(path, blocked_path) for blocked_path in blocked_paths)
elif reason == "not_created_or_allowed":
Expand All @@ -456,6 +458,8 @@ def test_is_allowed_file_fuzzer(

if reason == "allowed":
assert any(is_in_or_equal(path, allowed_path) for allowed_path in allowed_paths)
elif reason == "created":
assert any(is_in_or_equal(path, created_path) for created_path in created_paths)


@pytest.mark.parametrize(
Expand All @@ -469,7 +473,7 @@ def test_is_allowed_file_fuzzer(
],
)
def is_allowed_file_corner_cases(path, blocked_paths, allowed_paths, result):
assert is_allowed_file(path, blocked_paths, allowed_paths) == result
assert is_allowed_file(path, blocked_paths, allowed_paths, []) == result


# Additional test for known edge cases
Expand Down

0 comments on commit 61f794b

Please sign in to comment.