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

Do not attach content_disposition_type = "attachment" headers for files explicitly allowed by developer #9348

Merged
merged 8 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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":
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice makes sense

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
Loading