From 9da752899e8106ba752670f16689ea76006d4faa Mon Sep 17 00:00:00 2001 From: Deepak kudi Date: Thu, 4 Jun 2026 09:06:45 +0530 Subject: [PATCH 1/6] Preserve upload handler bound args --- packages/reflex-base/news/5290.bugfix.md | 1 + .../src/reflex_base/event/__init__.py | 25 +++++++++++++------ tests/units/components/core/test_upload.py | 19 ++++++++++++++ 3 files changed, 37 insertions(+), 8 deletions(-) create mode 100644 packages/reflex-base/news/5290.bugfix.md diff --git a/packages/reflex-base/news/5290.bugfix.md b/packages/reflex-base/news/5290.bugfix.md new file mode 100644 index 00000000000..96e7e8ed5e5 --- /dev/null +++ b/packages/reflex-base/news/5290.bugfix.md @@ -0,0 +1 @@ +Preserve extra bound event arguments when `rx.upload_files` is used in an upload handler. diff --git a/packages/reflex-base/src/reflex_base/event/__init__.py b/packages/reflex-base/src/reflex_base/event/__init__.py index 842caeae089..6721536db1b 100644 --- a/packages/reflex-base/src/reflex_base/event/__init__.py +++ b/packages/reflex-base/src/reflex_base/event/__init__.py @@ -471,27 +471,36 @@ def __call__(self, *args: Any, **kwargs: Any) -> "EventSpec": raise EventHandlerTypeError(msg) fn_args = fn_args[: len(args)] + list(kwargs) - - fn_args = (Var(_js_expr=arg) for arg in fn_args) + event_args = [*args, *kwargs.values()] # Construct the payload. - values = [] - for arg in [*args, *kwargs.values()]: + payload = [] + upload_event_spec = None + for fn_arg, arg in zip(fn_args, event_args, strict=False): # Special case for file uploads. if isinstance(arg, (FileUpload, UploadFilesChunk)): - return arg.as_event_spec(handler=self) + if upload_event_spec is not None: + msg = ( + f"Event handler {self.fn.__name__} received multiple file " + "upload arguments." + ) + raise EventHandlerTypeError(msg) + upload_event_spec = arg.as_event_spec(handler=self) + continue # Otherwise, convert to JSON. try: - values.append(LiteralVar.create(arg)) + payload.append((Var(_js_expr=fn_arg), LiteralVar.create(arg))) except TypeError as e: msg = f"Arguments to event handlers must be Vars or JSON-serializable. Got {arg} of type {type(arg)}." raise EventHandlerTypeError(msg) from e - payload = tuple(zip(fn_args, values, strict=False)) + + if upload_event_spec is not None: + return upload_event_spec.with_args(upload_event_spec.args + tuple(payload)) # Return the event spec. return EventSpec( - handler=self, args=payload, event_actions=self.event_actions.copy() + handler=self, args=tuple(payload), event_actions=self.event_actions.copy() ) diff --git a/tests/units/components/core/test_upload.py b/tests/units/components/core/test_upload.py index 2bb990bac56..7a2f563601c 100644 --- a/tests/units/components/core/test_upload.py +++ b/tests/units/components/core/test_upload.py @@ -42,6 +42,10 @@ def not_drop_handler(self, not_files: Any): async def upload_alias_handler(self, uploads: list[rx.UploadFile]): """Handle uploaded files with a non-default parameter name.""" + @event + async def upload_with_field(self, files: list[rx.UploadFile], field: str): + """Handle uploaded files for a specific field.""" + class StreamingUploadStateTest(State): """Test state for streaming uploads.""" @@ -222,6 +226,21 @@ def test_upload_files_event_spec_carries_upload_provider_app_wrap(): ) +def test_upload_files_preserves_bound_event_args(): + field = Var(_js_expr="field", _var_type=str) + spec = cast( + EventSpec, + UploadStateTest.upload_with_field( + cast(Any, rx.upload_files(upload_id="foo_id")), + cast(Any, field), + ), + ) + arg_values = {arg[0]._js_expr: arg[1]._js_expr for arg in spec.args} + + assert arg_values["field"] == "field" + assert isinstance(Upload.create(id="foo_id", on_drop=spec), Upload) + + def test_styled_upload_create(): styled_up_comp_1 = StyledUpload.create() assert isinstance(styled_up_comp_1, StyledUpload) From 18297506becd33c6cb5101b61dbf8e1bb598e9af Mon Sep 17 00:00:00 2001 From: Farhan Ali Raza Date: Thu, 4 Jun 2026 23:58:16 +0500 Subject: [PATCH 2/6] fix: forward bound upload handler args to the backend Args bound to an upload handler (e.g. State.on_drop(rx.upload_files(...), field)) were preserved in the compiled event spec but never reached the backend handler, since uploads travel over a REST endpoint instead of the socket. The client now forwards the named extra args via a URL-encoded JSON header, which the upload endpoint decodes and merges into the event payload. Bound names that clash with reserved upload keys are rejected at build time. Fixes #5290. --- .../.templates/web/utils/helpers/upload.js | 11 ++- .../reflex_base/.templates/web/utils/state.js | 7 ++ .../src/reflex_base/event/__init__.py | 36 +++++++++- .../reflex_components_core/core/_upload.py | 26 ++++++- tests/integration/test_upload.py | 71 ++++++++++++++++++- tests/units/components/core/test_upload.py | 36 ++++++++++ 6 files changed, 179 insertions(+), 8 deletions(-) diff --git a/packages/reflex-base/src/reflex_base/.templates/web/utils/helpers/upload.js b/packages/reflex-base/src/reflex_base/.templates/web/utils/helpers/upload.js index 6d3d146c6c5..23212fd91ce 100644 --- a/packages/reflex-base/src/reflex_base/.templates/web/utils/helpers/upload.js +++ b/packages/reflex-base/src/reflex_base/.templates/web/utils/helpers/upload.js @@ -8,8 +8,9 @@ import env from "$/env.json"; * @param handler The handler to use. * @param upload_id The upload id to use. * @param on_upload_progress The function to call on upload progress. - * @param socket the websocket connection * @param extra_headers Extra headers to send with the request. + * @param extra_args Extra bound handler args to forward to the backend handler. + * @param socket the websocket connection * @param refs The refs object to store the abort controller in. * @param getBackendURL Function to get the backend URL. * @param getToken Function to get the Reflex token. @@ -22,6 +23,7 @@ export const uploadFiles = async ( upload_id, on_upload_progress, extra_headers, + extra_args, socket, refs, getBackendURL, @@ -147,6 +149,13 @@ export const uploadFiles = async ( xhr.open("POST", getBackendURL(env.UPLOAD)); xhr.setRequestHeader("Reflex-Client-Token", getToken()); xhr.setRequestHeader("Reflex-Event-Handler", handler); + if (extra_args && Object.keys(extra_args).length > 0) { + // URL-encode the JSON so arbitrary values are safe in a single HTTP header. + xhr.setRequestHeader( + "Reflex-Event-Args", + encodeURIComponent(JSON.stringify(extra_args)), + ); + } for (const [key, value] of Object.entries(extra_headers || {})) { xhr.setRequestHeader(key, value); } diff --git a/packages/reflex-base/src/reflex_base/.templates/web/utils/state.js b/packages/reflex-base/src/reflex_base/.templates/web/utils/state.js index c07b4dfcbac..9ca3123e732 100644 --- a/packages/reflex-base/src/reflex_base/.templates/web/utils/state.js +++ b/packages/reflex-base/src/reflex_base/.templates/web/utils/state.js @@ -420,6 +420,12 @@ export const applyEvent = async (event, socket, navigate, params) => { */ export const applyRestEvent = async (event, socket, navigate, params) => { if (event.handler === "uploadFiles") { + // The compiled event names its extra bound handler args; collect just those + // so they reach the backend handler (no need to know the reserved keys). + const extra_args = {}; + for (const name of event.payload.__reflex_event_arg_names ?? []) { + extra_args[name] = event.payload[name]; + } // Start upload, but do not wait for it, which would block other events. uploadFiles( event.name, @@ -427,6 +433,7 @@ export const applyRestEvent = async (event, socket, navigate, params) => { event.payload.upload_id, event.payload.on_upload_progress, event.payload.extra_headers, + extra_args, socket, refs, getBackendURL, diff --git a/packages/reflex-base/src/reflex_base/event/__init__.py b/packages/reflex-base/src/reflex_base/event/__init__.py index 6721536db1b..aa560c9223b 100644 --- a/packages/reflex-base/src/reflex_base/event/__init__.py +++ b/packages/reflex-base/src/reflex_base/event/__init__.py @@ -179,6 +179,13 @@ def from_event_type( EVENT_ACTIONS_MARKER = "_rx_event_actions" UPLOAD_FILES_CLIENT_HANDLER = "uploadFiles" +# Payload key listing the names of the extra bound handler args in an upload +# event. Uploads use a REST endpoint instead of the socket; the args stay flat +# (so event-arg validation still sees them) and this manifest tells the client +# uploadFiles handler which ones to forward, without it hardcoding the reserved +# upload keys. Kept in sync with the matching literal in the web template. +UPLOAD_EVENT_ARG_NAMES_KEY = "__reflex_event_arg_names" + def _handler_name(handler: "EventHandler") -> str: """Get a stable fully qualified handler name for errors. @@ -477,7 +484,9 @@ def __call__(self, *args: Any, **kwargs: Any) -> "EventSpec": payload = [] upload_event_spec = None for fn_arg, arg in zip(fn_args, event_args, strict=False): - # Special case for file uploads. + # Special case for file uploads. The upload arg takes its own + # positional slot so the remaining args stay aligned with fn_args, + # but its parameter name is re-derived server-side in as_event_spec. if isinstance(arg, (FileUpload, UploadFilesChunk)): if upload_event_spec is not None: msg = ( @@ -496,7 +505,30 @@ def __call__(self, *args: Any, **kwargs: Any) -> "EventSpec": raise EventHandlerTypeError(msg) from e if upload_event_spec is not None: - return upload_event_spec.with_args(upload_event_spec.args + tuple(payload)) + if not payload: + return upload_event_spec + # The extra bound args share the flat payload with the synthetic + # upload args, so reject names that would clobber a reserved upload + # key (files, upload_id, extra_headers, ...). + payload_names = [name._js_expr for name, _ in payload] + reserved = {name._js_expr for name, _ in upload_event_spec.args} + clash = next((name for name in payload_names if name in reserved), None) + if clash is not None: + msg = ( + f"Event handler {self.fn.__name__} argument {clash!r} conflicts " + "with a reserved upload argument." + ) + raise EventHandlerTypeError(msg) + # The client uploadFiles handler forwards exactly the args named here, + # so it never has to know the reserved upload keys. + return upload_event_spec.with_args(( + *upload_event_spec.args, + *payload, + ( + Var(_js_expr=UPLOAD_EVENT_ARG_NAMES_KEY), + LiteralVar.create(payload_names), + ), + )) # Return the event spec. return EventSpec( diff --git a/packages/reflex-components-core/src/reflex_components_core/core/_upload.py b/packages/reflex-components-core/src/reflex_components_core/core/_upload.py index 680fd7c613f..61282c45293 100644 --- a/packages/reflex-components-core/src/reflex_components_core/core/_upload.py +++ b/packages/reflex-components-core/src/reflex_components_core/core/_upload.py @@ -5,10 +5,12 @@ import asyncio import contextlib import dataclasses +import json from collections import deque from collections.abc import AsyncGenerator, AsyncIterator, MutableMapping from pathlib import Path from typing import TYPE_CHECKING, Any, BinaryIO, cast +from urllib.parse import unquote from python_multipart.multipart import MultipartParser, parse_options_header from reflex_base.utils import exceptions @@ -424,6 +426,25 @@ def _require_upload_headers(request: Request) -> tuple[str, str]: return token, handler +def _extra_upload_args(request: Request) -> dict[str, Any]: + """Decode extra bound handler args sent alongside an upload. + + Uploads travel over a dedicated REST endpoint, so any args bound to the + handler (e.g. ``State.on_drop(rx.upload_files(...), field)``) are forwarded + as a URL-encoded JSON header rather than the normal event payload. + + Args: + request: The incoming upload request. + + Returns: + The decoded extra args, or an empty mapping if none were sent. + """ + encoded = request.headers.get("reflex-event-args") + if not encoded: + return {} + return json.loads(unquote(encoded)) + + async def _upload_buffered_file( request: Request, app: App, @@ -447,6 +468,7 @@ async def _upload_buffered_file( except ClientDisconnect: return Response() + extra_args = _extra_upload_args(request) form_data_closed = False async def _close_form_data() -> None: @@ -481,7 +503,7 @@ def _create_upload_event() -> Event: return Event( name=handler_name, - payload={handler_upload_param[0]: file_uploads}, + payload={**extra_args, handler_upload_param[0]: file_uploads}, ) event: Event | None = None @@ -555,7 +577,7 @@ async def _upload_chunk_file( chunk_iter = UploadChunkIterator(maxsize=8) event = Event( name=handler_name, - payload={handler_upload_param[0]: chunk_iter}, + payload={**_extra_upload_args(request), handler_upload_param[0]: chunk_iter}, ) task_future = await app.event_processor.enqueue(token, event) diff --git a/tests/integration/test_upload.py b/tests/integration/test_upload.py index 11e34609302..4c64761e82e 100644 --- a/tests/integration/test_upload.py +++ b/tests/integration/test_upload.py @@ -37,8 +37,10 @@ class UploadState(rx.State): disabled: rx.Field[bool] = rx.field(False) large_data: rx.Field[str] = rx.field("") quaternary_names: rx.Field[list[str]] = rx.field([]) + quaternary_field: rx.Field[str] = rx.field("") stream_chunk_records: rx.Field[list[str]] = rx.field([]) stream_completed_files: rx.Field[list[str]] = rx.field([]) + stream_field: rx.Field[str] = rx.field("") @rx.event async def handle_upload(self, files: list[rx.UploadFile]): @@ -94,15 +96,21 @@ async def handle_upload_tertiary(self, files: list[rx.UploadFile]): self.upload_done = True @rx.event - async def handle_upload_quaternary(self, files: list[rx.UploadFile]): + async def handle_upload_quaternary( + self, files: list[rx.UploadFile], field: str + ): self.upload_done = False self.quaternary_names = [file.name for file in files if file.name] + self.quaternary_field = field self.upload_done = True @rx.event(background=True) - async def handle_upload_stream(self, chunk_iter: rx.UploadChunkIterator): + async def handle_upload_stream( + self, chunk_iter: rx.UploadChunkIterator, field: str + ): async with self: self.upload_done = False + self.stream_field = field upload_dir = rx.get_upload_dir() / "streaming" file_handles: dict[str, Any] = {} @@ -260,6 +268,7 @@ def index(): rx.upload_files( # pyright: ignore [reportArgumentType] upload_id="quaternary", ), + "resume-field", ), id="quaternary", ), @@ -267,6 +276,11 @@ def index(): UploadState.quaternary_names.to_string(), id="quaternary_files", ), + rx.input( + value=UploadState.quaternary_field, + read_only=True, + id="quaternary_field", + ), rx.heading("Streaming Upload"), rx.upload.root( rx.vstack( @@ -281,7 +295,8 @@ def index(): rx.upload_files_chunk( # pyright: ignore [reportArgumentType] upload_id="streaming", on_upload_progress=UploadState.stream_upload_progress, - ) + ), + "stream-field", ), id="upload_button_streaming", ), @@ -305,6 +320,11 @@ def index(): UploadState.stream_completed_files.to_string(), id="stream_completed_files", ), + rx.input( + value=UploadState.stream_field, + read_only=True, + id="stream_field", + ), rx.vstack( rx.foreach( UploadState.stream_progress_dicts, @@ -661,6 +681,47 @@ async def test_upload_file_multiple(tmp_path, upload_file: AppHarness, driver): assert actual_contents == exp_content +def test_upload_file_with_bound_arg( + tmp_path, upload_file: AppHarness, driver: WebDriver +): + """Upload via an on_drop handler bound with an extra arg and verify it arrives. + + Regression test for https://github.com/reflex-dev/reflex/issues/5290: extra + args bound to an upload handler must reach the backend handler, not just the + compiled event spec. + + Args: + tmp_path: pytest tmp_path fixture. + upload_file: harness for UploadFile app. + driver: WebDriver instance. + """ + assert upload_file.app_instance is not None + poll_for_token(driver, upload_file) + clear_btn = driver.find_element(By.ID, "clear_uploads") + clear_btn.click() + + upload_box = get_upload_box(driver, upload_root_id="quaternary") + assert upload_box + + exp_name = "bound_arg.txt" + target_file = tmp_path / exp_name + target_file.write_text("bound arg upload contents!") + + # Selecting a file fires on_drop, which carries the bound "resume-field" arg. + upload_box.send_keys(str(target_file)) + + upload_done = driver.find_element(By.ID, "upload_done") + assert upload_file.poll_for_value(upload_done, exp_not_equal="false") == "true" + + # The bound arg must have reached the handler. + field_display = driver.find_element(By.ID, "quaternary_field") + assert upload_file.poll_for_value(field_display, exp_not_equal="") == "resume-field" + + # The uploaded file itself must still arrive. + names_display = driver.find_element(By.ID, "quaternary_files") + assert Path(exp_name).name in names_display.text + + @pytest.mark.parametrize("upload_root_id", [None, "secondary"]) def test_clear_files( tmp_path, upload_file: AppHarness, driver: WebDriver, upload_root_id: str | None @@ -852,6 +913,10 @@ async def test_upload_chunk_file(tmp_path, upload_file: AppHarness, driver: WebD upload_done = driver.find_element(By.ID, "upload_done") assert upload_file.poll_for_value(upload_done, exp_not_equal="false") == "true" + # The bound arg must reach the streaming handler too. + stream_field = driver.find_element(By.ID, "stream_field") + assert upload_file.poll_for_value(stream_field, exp_not_equal="") == "stream-field" + for exp_name, exp_contents in exp_files.items(): assert ( rx.get_upload_dir() / "streaming" / exp_name diff --git a/tests/units/components/core/test_upload.py b/tests/units/components/core/test_upload.py index 7a2f563601c..5b5861b7e0b 100644 --- a/tests/units/components/core/test_upload.py +++ b/tests/units/components/core/test_upload.py @@ -46,6 +46,12 @@ async def upload_alias_handler(self, uploads: list[rx.UploadFile]): async def upload_with_field(self, files: list[rx.UploadFile], field: str): """Handle uploaded files for a specific field.""" + @event + async def upload_with_reserved_arg( + self, files: list[rx.UploadFile], upload_id: str + ): + """Handle uploaded files with a bound arg that shadows a reserved key.""" + class StreamingUploadStateTest(State): """Test state for streaming uploads.""" @@ -226,6 +232,12 @@ def test_upload_files_event_spec_carries_upload_provider_app_wrap(): ) +# Matches UPLOAD_EVENT_ARG_NAMES_KEY in reflex_base.event and the web template. +# The constant isn't part of the module's public import surface (event/__init__.py +# proxies attribute access through EventNamespace), so it's mirrored here. +UPLOAD_EVENT_ARG_NAMES_KEY = "__reflex_event_arg_names" + + def test_upload_files_preserves_bound_event_args(): field = Var(_js_expr="field", _var_type=str) spec = cast( @@ -237,10 +249,34 @@ def test_upload_files_preserves_bound_event_args(): ) arg_values = {arg[0]._js_expr: arg[1]._js_expr for arg in spec.args} + # The bound arg stays a flat payload entry (keyed by its param name) and is + # advertised in the manifest so the client knows to forward it. assert arg_values["field"] == "field" + assert "field" in arg_values[UPLOAD_EVENT_ARG_NAMES_KEY] assert isinstance(Upload.create(id="foo_id", on_drop=spec), Upload) +def test_upload_files_multiple_upload_args_raises(): + from reflex_base.utils.exceptions import EventHandlerTypeError + + with pytest.raises(EventHandlerTypeError, match="multiple file upload arguments"): + UploadStateTest.upload_with_field( + cast(Any, rx.upload_files(upload_id="foo_id")), + cast(Any, rx.upload_files(upload_id="bar_id")), + ) + + +def test_upload_files_bound_arg_reserved_name_raises(): + """A bound arg sharing a reserved upload key name is rejected at build time.""" + from reflex_base.utils.exceptions import EventHandlerTypeError + + with pytest.raises(EventHandlerTypeError, match="reserved upload argument"): + UploadStateTest.upload_with_reserved_arg( + cast(Any, rx.upload_files(upload_id="foo_id")), + cast(Any, Var(_js_expr="some_id", _var_type=str)), + ) + + def test_styled_upload_create(): styled_up_comp_1 = StyledUpload.create() assert isinstance(styled_up_comp_1, StyledUpload) From c703c8dff00c06dc91c03684051e8a9bc8f9af86 Mon Sep 17 00:00:00 2001 From: Farhan Ali Raza Date: Fri, 5 Jun 2026 00:47:58 +0500 Subject: [PATCH 3/6] docs: add changelog for upload bound-arg fix (#5290) --- packages/reflex-components-core/news/5290.bugfix.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 packages/reflex-components-core/news/5290.bugfix.md diff --git a/packages/reflex-components-core/news/5290.bugfix.md b/packages/reflex-components-core/news/5290.bugfix.md new file mode 100644 index 00000000000..8d24e4a0c3e --- /dev/null +++ b/packages/reflex-components-core/news/5290.bugfix.md @@ -0,0 +1 @@ +Deliver extra bound handler arguments to upload handlers, so `on_drop=State.handle_upload(rx.upload_files(...), field)` passes `field` through to the backend instead of raising a missing-argument error. From f29dd0bce21429b9c76b7af13b7a9a30dd4d0408 Mon Sep 17 00:00:00 2001 From: Farhan Ali Raza Date: Sat, 6 Jun 2026 02:07:54 +0500 Subject: [PATCH 4/6] fix: return 400 on malformed reflex-event-args upload header Validate the reflex-event-args header before parsing form data so a malformed or non-object JSON payload yields a 400 instead of a 500. --- .../reflex_components_core/core/_upload.py | 19 ++++++- tests/units/components/core/test_upload.py | 52 +++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/packages/reflex-components-core/src/reflex_components_core/core/_upload.py b/packages/reflex-components-core/src/reflex_components_core/core/_upload.py index 61282c45293..6d94e93a115 100644 --- a/packages/reflex-components-core/src/reflex_components_core/core/_upload.py +++ b/packages/reflex-components-core/src/reflex_components_core/core/_upload.py @@ -438,11 +438,25 @@ def _extra_upload_args(request: Request) -> dict[str, Any]: Returns: The decoded extra args, or an empty mapping if none were sent. + + Raises: + HTTPException: If the header is present but not a valid JSON object. """ encoded = request.headers.get("reflex-event-args") if not encoded: return {} - return json.loads(unquote(encoded)) + try: + decoded = json.loads(unquote(encoded)) + except json.JSONDecodeError as exc: + raise HTTPException( + status_code=400, detail="Malformed reflex-event-args header." + ) from exc + if not isinstance(decoded, dict): + raise HTTPException( + status_code=400, + detail="reflex-event-args header must encode a JSON object.", + ) + return decoded async def _upload_buffered_file( @@ -463,12 +477,13 @@ async def _upload_buffered_file( from reflex.state import StateUpdate + extra_args = _extra_upload_args(request) + try: form_data = await request.form() except ClientDisconnect: return Response() - extra_args = _extra_upload_args(request) form_data_closed = False async def _close_form_data() -> None: diff --git a/tests/units/components/core/test_upload.py b/tests/units/components/core/test_upload.py index 5b5861b7e0b..b3ea6bf585e 100644 --- a/tests/units/components/core/test_upload.py +++ b/tests/units/components/core/test_upload.py @@ -1,9 +1,12 @@ +import json from typing import Any, cast +from urllib.parse import quote import pytest from reflex_base.event import EventChain, EventHandler, EventSpec, parse_args_spec from reflex_base.vars import VarData from reflex_base.vars.base import LiteralVar, Var +from reflex_components_core.core._upload import _extra_upload_args from reflex_components_core.core.upload import ( GhostUpload, StyledUpload, @@ -13,6 +16,8 @@ cancel_upload, get_upload_url, ) +from starlette.exceptions import HTTPException +from starlette.requests import Request import reflex as rx from reflex import event @@ -277,6 +282,53 @@ def test_upload_files_bound_arg_reserved_name_raises(): ) +@pytest.fixture +def upload_request(): + """Build an upload request carrying a given reflex-event-args header value. + + Returns: + A factory taking the header value (or ``None`` to omit the header). + """ + + def _build(header_value: str | None) -> Request: + headers = ( + [(b"reflex-event-args", header_value.encode())] + if header_value is not None + else [] + ) + return Request({"type": "http", "headers": headers}) + + return _build + + +def test_extra_upload_args_decodes_json_object(upload_request): + request = upload_request(quote(json.dumps({"field": "value"}))) + assert _extra_upload_args(request) == {"field": "value"} + + +def test_extra_upload_args_missing_header_returns_empty(upload_request): + assert _extra_upload_args(upload_request(None)) == {} + assert _extra_upload_args(upload_request("")) == {} + + +@pytest.mark.parametrize( + "header_value", + [quote("[1, 2, 3]"), quote('"foo"'), quote("42"), quote("null")], +) +def test_extra_upload_args_non_object_raises(upload_request, header_value): + """A header encoding valid JSON that is not an object is a bad request.""" + with pytest.raises(HTTPException) as exc_info: + _extra_upload_args(upload_request(header_value)) + assert exc_info.value.status_code == 400 + + +def test_extra_upload_args_malformed_json_raises(upload_request): + """A header that is not valid JSON is a bad request, not a 500.""" + with pytest.raises(HTTPException) as exc_info: + _extra_upload_args(upload_request(quote("{not json"))) + assert exc_info.value.status_code == 400 + + def test_styled_upload_create(): styled_up_comp_1 = StyledUpload.create() assert isinstance(styled_up_comp_1, StyledUpload) From 4ce64060160c73cb055024eff52c0f15ac405bcb Mon Sep 17 00:00:00 2001 From: Farhan Ali Raza Date: Sat, 6 Jun 2026 03:42:53 +0500 Subject: [PATCH 5/6] fix: carry upload bound args as a form field so streaming uploads get them Bound handler args were sent in a URL-encoded HTTP header, which the streaming chunk parser never read, so chunked uploads dropped them and the header path was capped by server limits. Move the args into a leading __reflex_event_args multipart field parsed ahead of the first file chunk, dispatching the handler once it is read. The field is size-bounded and rejected if it arrives after a file part. --- .../.templates/web/utils/helpers/upload.js | 14 +- .../reflex_components_core/core/_upload.py | 155 ++++++++++--- tests/units/components/core/test_upload.py | 205 +++++++++++++++--- 3 files changed, 301 insertions(+), 73 deletions(-) diff --git a/packages/reflex-base/src/reflex_base/.templates/web/utils/helpers/upload.js b/packages/reflex-base/src/reflex_base/.templates/web/utils/helpers/upload.js index 23212fd91ce..17b94c1fa51 100644 --- a/packages/reflex-base/src/reflex_base/.templates/web/utils/helpers/upload.js +++ b/packages/reflex-base/src/reflex_base/.templates/web/utils/helpers/upload.js @@ -76,6 +76,13 @@ export const uploadFiles = async ( const controller = new AbortController(); const formdata = new FormData(); + // Bound handler args are sent as a JSON form field that must precede the + // files, so the streaming chunk parser reads it before the first file part. + // Field name kept in sync with UPLOAD_EVENT_ARGS_FIELD in _upload.py. + if (extra_args && Object.keys(extra_args).length > 0) { + formdata.append("__reflex_event_args", JSON.stringify(extra_args)); + } + // Add the token and handler to the file name. files.forEach((file) => { formdata.append("files", file, file.path || file.name); @@ -149,13 +156,6 @@ export const uploadFiles = async ( xhr.open("POST", getBackendURL(env.UPLOAD)); xhr.setRequestHeader("Reflex-Client-Token", getToken()); xhr.setRequestHeader("Reflex-Event-Handler", handler); - if (extra_args && Object.keys(extra_args).length > 0) { - // URL-encode the JSON so arbitrary values are safe in a single HTTP header. - xhr.setRequestHeader( - "Reflex-Event-Args", - encodeURIComponent(JSON.stringify(extra_args)), - ); - } for (const [key, value] of Object.entries(extra_headers || {})) { xhr.setRequestHeader(key, value); } diff --git a/packages/reflex-components-core/src/reflex_components_core/core/_upload.py b/packages/reflex-components-core/src/reflex_components_core/core/_upload.py index 6d94e93a115..423bc83403c 100644 --- a/packages/reflex-components-core/src/reflex_components_core/core/_upload.py +++ b/packages/reflex-components-core/src/reflex_components_core/core/_upload.py @@ -7,16 +7,21 @@ import dataclasses import json from collections import deque -from collections.abc import AsyncGenerator, AsyncIterator, MutableMapping +from collections.abc import ( + AsyncGenerator, + AsyncIterator, + Awaitable, + Callable, + MutableMapping, +) from pathlib import Path from typing import TYPE_CHECKING, Any, BinaryIO, cast -from urllib.parse import unquote from python_multipart.multipart import MultipartParser, parse_options_header from reflex_base.utils import exceptions from reflex_base.utils.format import json_dumps from reflex_base.utils.streaming_response import DisconnectAwareStreamingResponse -from starlette.datastructures import Headers +from starlette.datastructures import FormData, Headers from starlette.datastructures import UploadFile as StarletteUploadFile from starlette.exceptions import HTTPException from starlette.formparsers import MultiPartException, _user_safe_decode @@ -233,6 +238,7 @@ class _UploadChunkPart: offset: int = 0 bytes_emitted: int = 0 is_upload_chunk: bool = False + is_text_field: bool = False @dataclasses.dataclass(kw_only=True, slots=True) @@ -242,6 +248,9 @@ class _UploadChunkMultipartParser: headers: Headers stream: AsyncGenerator[bytes, None] chunk_iter: UploadChunkIterator + # Dispatched with the raw bound-args field once it is parsed (before any file + # chunk is pushed), or at end-of-parse if the upload carried no files. + on_args_ready: Callable[[str | None], Awaitable[None]] _charset: str = "" _current_partial_header_name: bytes = b"" _current_partial_header_value: bytes = b"" @@ -249,6 +258,9 @@ class _UploadChunkMultipartParser: default_factory=_UploadChunkPart ) _chunks_to_emit: deque[UploadChunk] = dataclasses.field(default_factory=deque) + _args_buffer: bytearray = dataclasses.field(default_factory=bytearray) + _extra_args_raw: str | None = None + _started: bool = False _seen_upload_chunk: bool = False _part_count: int = 0 _emitted_chunk_count: int = 0 @@ -261,6 +273,12 @@ def on_part_begin(self) -> None: def on_part_data(self, data: bytes, start: int, end: int) -> None: """Record streamed chunk data for the current part.""" + if self._current_part.is_text_field: + self._args_buffer += data[start:end] + if len(self._args_buffer) > MAX_UPLOAD_EVENT_ARGS_BYTES: + msg = "Upload event args field is too large." + raise MultiPartException(msg) + return if ( not self._current_part.is_upload_chunk or self._current_part.filename is None @@ -281,7 +299,12 @@ def on_part_data(self, data: bytes, start: int, end: int) -> None: self._emitted_bytes += len(message_bytes) def on_part_end(self) -> None: - """Emit a zero-byte chunk for empty file parts.""" + """Finalize the bound-args field, or emit a zero-byte chunk for empty files.""" + if self._current_part.is_text_field: + # Use the same charset-tolerant decode as the file/field-name parsing + # so a bogus request charset falls back instead of raising LookupError. + self._extra_args_raw = _user_safe_decode(self._args_buffer, self._charset) + return if ( self._current_part.is_upload_chunk and self._current_part.filename is not None @@ -332,11 +355,23 @@ def on_headers_finished(self) -> None: msg = 'The Content-Disposition header field "name" must be provided.' raise MultiPartException(msg) from err - try: - filename = _user_safe_decode(options[b"filename"], self._charset) - except KeyError: - # Ignore non-file form fields entirely. + if b"filename" not in options: + # Capture the bound-args field; ignore any other non-file field. + if field_name == UPLOAD_EVENT_ARGS_FIELD: + if self._seen_upload_chunk: + # The handler is dispatched at the first file part, so a late + # args field would be silently dropped; reject it loudly. + msg = "Upload event args must precede the file parts." + raise MultiPartException(msg) + self._current_part.is_text_field = True + self._args_buffer = bytearray() return + if field_name == UPLOAD_EVENT_ARGS_FIELD: + # A file under the args field name would otherwise be pushed as a + # phantom file upload; reject it instead of silently dropping it. + msg = "Upload event args must be a text field, not a file." + raise MultiPartException(msg) + filename = _user_safe_decode(options[b"filename"], self._charset) filename = Path(filename.lstrip("/")).name content_type = "" @@ -351,6 +386,8 @@ def on_headers_finished(self) -> None: self._current_part.offset = 0 self._current_part.bytes_emitted = 0 self._current_part.is_upload_chunk = True + # The args field precedes the files, so by the first file part the bound + # args are fully parsed and the handler can be dispatched. self._seen_upload_chunk = True self._part_count += 1 @@ -362,12 +399,27 @@ async def _flush_emitted_chunks(self) -> None: while self._chunks_to_emit: await self.chunk_iter.push(self._chunks_to_emit.popleft()) + async def _maybe_start_handler(self, *, force: bool = False) -> None: + """Dispatch the handler once the bound args are parsed. + + Called before chunks are flushed so the consumer task is registered + ahead of the first pushed chunk. + + Args: + force: Dispatch even if no file part was seen (args-only/empty upload). + """ + if self._started or not (self._seen_upload_chunk or force): + return + self._started = True + await self.on_args_ready(self._extra_args_raw) + async def parse(self) -> None: """Parse the incoming request stream and push chunks to the iterator. Raises: MultiPartException: If the request is not valid multipart upload data. RuntimeError: If the upload handler exits before consuming all chunks. + HTTPException: If the bound-args field is not a valid JSON object. """ _, params = parse_options_header(self.headers["Content-Type"]) charset = params.get(b"charset", "utf-8") @@ -396,9 +448,11 @@ async def parse(self) -> None: async for chunk in self.stream: self._stream_chunk_count += 1 parser.write(chunk) + await self._maybe_start_handler() await self._flush_emitted_chunks() parser.finalize() + await self._maybe_start_handler(force=True) await self._flush_emitted_chunks() @@ -426,39 +480,66 @@ def _require_upload_headers(request: Request) -> tuple[str, str]: return token, handler -def _extra_upload_args(request: Request) -> dict[str, Any]: - """Decode extra bound handler args sent alongside an upload. +# Multipart form field carrying the JSON-encoded extra bound handler args. +# Uploads travel over a REST endpoint instead of the socket, so args bound to +# the handler (e.g. ``State.on_drop(rx.upload_files(...), field)``) ride in this +# field. Kept in sync with the matching literal in the web upload template. +UPLOAD_EVENT_ARGS_FIELD = "__reflex_event_args" + +# Cap on the buffered bound-args field for streaming uploads. The args are small +# identifiers, so this only bounds the in-memory buffer against a hostile client +# (file parts are backpressured via the chunk iterator; this field is not). +MAX_UPLOAD_EVENT_ARGS_BYTES = 1024 * 1024 - Uploads travel over a dedicated REST endpoint, so any args bound to the - handler (e.g. ``State.on_drop(rx.upload_files(...), field)``) are forwarded - as a URL-encoded JSON header rather than the normal event payload. + +def _decode_event_args(encoded: str | None) -> dict[str, Any]: + """Decode the extra bound handler args sent alongside an upload. Args: - request: The incoming upload request. + encoded: The JSON-encoded args form field value, or ``None`` if absent. Returns: The decoded extra args, or an empty mapping if none were sent. Raises: - HTTPException: If the header is present but not a valid JSON object. + HTTPException: If the value is present but not a valid JSON object. """ - encoded = request.headers.get("reflex-event-args") if not encoded: return {} try: - decoded = json.loads(unquote(encoded)) + decoded = json.loads(encoded) except json.JSONDecodeError as exc: raise HTTPException( - status_code=400, detail="Malformed reflex-event-args header." + status_code=400, detail="Malformed upload event args." ) from exc if not isinstance(decoded, dict): raise HTTPException( - status_code=400, - detail="reflex-event-args header must encode a JSON object.", + status_code=400, detail="Upload event args must be a JSON object." ) return decoded +def _buffered_upload_args(form_data: FormData) -> dict[str, Any]: + """Decode the bound handler args from a buffered upload's form data. + + Args: + form_data: The parsed multipart form data. + + Returns: + The decoded extra args, or an empty mapping if none were sent. + + Raises: + HTTPException: If the args field is a file or not a valid JSON object. + """ + raw_args = form_data.get(UPLOAD_EVENT_ARGS_FIELD) + if raw_args is not None and not isinstance(raw_args, str): + raise HTTPException( + status_code=400, + detail="Upload event args must be a text field, not a file.", + ) + return _decode_event_args(raw_args) + + async def _upload_buffered_file( request: Request, app: App, @@ -477,8 +558,6 @@ async def _upload_buffered_file( from reflex.state import StateUpdate - extra_args = _extra_upload_args(request) - try: form_data = await request.form() except ClientDisconnect: @@ -500,6 +579,7 @@ def _create_upload_event() -> Event: Returns: The upload event backed by the parsed files. """ + extra_args = _buffered_upload_args(form_data) files = form_data.getlist("files") file_uploads = [] for file in files: @@ -590,26 +670,39 @@ async def _upload_chunk_file( from reflex_base.event import Event chunk_iter = UploadChunkIterator(maxsize=8) - event = Event( - name=handler_name, - payload={**_extra_upload_args(request), handler_upload_param[0]: chunk_iter}, - ) - task_future = await app.event_processor.enqueue(token, event) + task_future: asyncio.Future[Any] | None = None + + async def _start_handler(extra_args_raw: str | None) -> None: + """Dispatch the streaming handler once the bound args are parsed. - chunk_iter.set_consumer_task(task_future) + Args: + extra_args_raw: The raw JSON args form field, or ``None`` if absent. + """ + nonlocal task_future + event = Event( + name=handler_name, + payload={ + **_decode_event_args(extra_args_raw), + handler_upload_param[0]: chunk_iter, + }, + ) + task_future = await app.event_processor.enqueue(token, event) + chunk_iter.set_consumer_task(task_future) parser = _UploadChunkMultipartParser( headers=request.headers, stream=request.stream(), chunk_iter=chunk_iter, + on_args_ready=_start_handler, ) try: await parser.parse() except ClientDisconnect: - task_future.cancel() - with contextlib.suppress(asyncio.CancelledError): - await task_future + if task_future is not None: + task_future.cancel() + with contextlib.suppress(asyncio.CancelledError): + await task_future return Response() except (MultiPartException, RuntimeError, ValueError) as err: await chunk_iter.fail(err) diff --git a/tests/units/components/core/test_upload.py b/tests/units/components/core/test_upload.py index b3ea6bf585e..eddef933789 100644 --- a/tests/units/components/core/test_upload.py +++ b/tests/units/components/core/test_upload.py @@ -1,12 +1,19 @@ +import asyncio +import io import json from typing import Any, cast -from urllib.parse import quote import pytest from reflex_base.event import EventChain, EventHandler, EventSpec, parse_args_spec from reflex_base.vars import VarData from reflex_base.vars.base import LiteralVar, Var -from reflex_components_core.core._upload import _extra_upload_args +from reflex_components_core.core._upload import ( + UPLOAD_EVENT_ARGS_FIELD, + UploadChunkIterator, + _buffered_upload_args, + _decode_event_args, + _UploadChunkMultipartParser, +) from reflex_components_core.core.upload import ( GhostUpload, StyledUpload, @@ -16,8 +23,10 @@ cancel_upload, get_upload_url, ) +from starlette.datastructures import FormData, Headers +from starlette.datastructures import UploadFile as StarletteUploadFile from starlette.exceptions import HTTPException -from starlette.requests import Request +from starlette.formparsers import MultiPartException import reflex as rx from reflex import event @@ -282,53 +291,179 @@ def test_upload_files_bound_arg_reserved_name_raises(): ) -@pytest.fixture -def upload_request(): - """Build an upload request carrying a given reflex-event-args header value. +def test_decode_event_args_decodes_json_object(): + assert _decode_event_args(json.dumps({"field": "value"})) == {"field": "value"} - Returns: - A factory taking the header value (or ``None`` to omit the header). - """ - def _build(header_value: str | None) -> Request: - headers = ( - [(b"reflex-event-args", header_value.encode())] - if header_value is not None - else [] - ) - return Request({"type": "http", "headers": headers}) +def test_decode_event_args_missing_returns_empty(): + assert _decode_event_args(None) == {} + assert _decode_event_args("") == {} - return _build +@pytest.mark.parametrize("encoded", ["[1, 2, 3]", '"foo"', "42", "null"]) +def test_decode_event_args_non_object_raises(encoded): + """A field encoding valid JSON that is not an object is a bad request.""" + with pytest.raises(HTTPException) as exc_info: + _decode_event_args(encoded) + assert exc_info.value.status_code == 400 -def test_extra_upload_args_decodes_json_object(upload_request): - request = upload_request(quote(json.dumps({"field": "value"}))) - assert _extra_upload_args(request) == {"field": "value"} +def test_decode_event_args_malformed_json_raises(): + """A field that is not valid JSON is a bad request, not a 500.""" + with pytest.raises(HTTPException) as exc_info: + _decode_event_args("{not json") + assert exc_info.value.status_code == 400 -def test_extra_upload_args_missing_header_returns_empty(upload_request): - assert _extra_upload_args(upload_request(None)) == {} - assert _extra_upload_args(upload_request("")) == {} +def test_buffered_upload_args_decodes_text_field(): + form = FormData([(UPLOAD_EVENT_ARGS_FIELD, json.dumps({"field": "value"}))]) + assert _buffered_upload_args(form) == {"field": "value"} -@pytest.mark.parametrize( - "header_value", - [quote("[1, 2, 3]"), quote('"foo"'), quote("42"), quote("null")], -) -def test_extra_upload_args_non_object_raises(upload_request, header_value): - """A header encoding valid JSON that is not an object is a bad request.""" - with pytest.raises(HTTPException) as exc_info: - _extra_upload_args(upload_request(header_value)) - assert exc_info.value.status_code == 400 +def test_buffered_upload_args_missing_returns_empty(): + assert _buffered_upload_args(FormData([])) == {} -def test_extra_upload_args_malformed_json_raises(upload_request): - """A header that is not valid JSON is a bad request, not a 500.""" + +def test_buffered_upload_args_file_rejected(): + """A file smuggled under the args field name is a bad request, not no-args.""" + upload = StarletteUploadFile(file=io.BytesIO(b"{}"), filename="args.json") + form = FormData([(UPLOAD_EVENT_ARGS_FIELD, upload)]) with pytest.raises(HTTPException) as exc_info: - _extra_upload_args(upload_request(quote("{not json"))) + _buffered_upload_args(form) assert exc_info.value.status_code == 400 +def _multipart_body( + boundary: str, *, args: str | None = None, file_first: bool = False +) -> bytes: + """Build a multipart upload body, optionally with a bound-args field. + + Args: + boundary: The multipart boundary token. + args: The raw bound-args field value, or ``None`` to omit it. + file_first: Place the file part before the args field (contract violation). + + Returns: + The encoded multipart body. + """ + args_part = ( + f"--{boundary}\r\n" + 'Content-Disposition: form-data; name="__reflex_event_args"\r\n\r\n' + f"{args}\r\n" + if args is not None + else "" + ) + file_part = ( + f"--{boundary}\r\n" + 'Content-Disposition: form-data; name="files"; filename="a.txt"\r\n' + "Content-Type: text/plain\r\n\r\n" + "hello\r\n" + ) + parts = [file_part, args_part] if file_first else [args_part, file_part] + return ("".join(parts) + f"--{boundary}--\r\n").encode() + + +async def _run_chunk_parser( + body: bytes, boundary: str, *, charset: str | None = None +) -> tuple[str | None, list[bytes]]: + """Drive the streaming chunk parser over a multipart body. + + Args: + body: The multipart request body. + boundary: The multipart boundary token. + charset: Optional charset to advertise in the request Content-Type. + + Returns: + The raw args field passed to dispatch and the file chunk payloads. + """ + + async def _stream(): + # Deliver in two writes so dispatch fires between parser.write calls. + mid = len(body) // 2 + for piece in (body[:mid], body[mid:]): + await asyncio.sleep(0) + yield piece + + args_received: asyncio.Queue[str | None] = asyncio.Queue() + + async def _on_args_ready(raw: str | None) -> None: + await args_received.put(raw) + + content_type = f"multipart/form-data; boundary={boundary}" + if charset is not None: + content_type += f"; charset={charset}" + + chunk_iter = UploadChunkIterator(maxsize=8) + parser = _UploadChunkMultipartParser( + headers=Headers({"content-type": content_type}), + stream=_stream(), + chunk_iter=chunk_iter, + on_args_ready=_on_args_ready, + ) + await parser.parse() + await chunk_iter.finish() + chunks = [chunk.data async for chunk in chunk_iter] + return await args_received.get(), chunks + + +async def test_chunk_parser_captures_bound_args_before_file(): + """The streaming parser captures the leading args field and emits the file.""" + raw, chunks = await _run_chunk_parser( + _multipart_body("BOUNDARY", args='{"field": "value"}'), "BOUNDARY" + ) + assert _decode_event_args(raw) == {"field": "value"} + assert b"".join(chunks) == b"hello" + + +async def test_chunk_parser_without_args_dispatches_empty(): + """A streaming upload with no args field still dispatches (with empty args).""" + raw, chunks = await _run_chunk_parser(_multipart_body("BOUNDARY"), "BOUNDARY") + assert _decode_event_args(raw) == {} + assert b"".join(chunks) == b"hello" + + +async def test_chunk_parser_bogus_charset_does_not_crash(): + """A bogus request charset must fall back, not raise LookupError -> 500.""" + raw, chunks = await _run_chunk_parser( + _multipart_body("BOUNDARY", args='{"field": "value"}'), + "BOUNDARY", + charset="bogus-cs", + ) + assert _decode_event_args(raw) == {"field": "value"} + assert b"".join(chunks) == b"hello" + + +async def test_chunk_parser_args_after_file_rejected(): + """Bound args sent after a file part are rejected, not silently dropped.""" + body = _multipart_body("BOUNDARY", args='{"field": "value"}', file_first=True) + with pytest.raises(MultiPartException): + await _run_chunk_parser(body, "BOUNDARY") + + +async def test_chunk_parser_args_as_file_rejected(): + """A file under the args field name is rejected, not treated as a phantom file.""" + body = ( + b"--BOUNDARY\r\n" + b'Content-Disposition: form-data; name="__reflex_event_args"; ' + b'filename="args.json"\r\n' + b"Content-Type: application/json\r\n\r\n" + b'{"field": "value"}\r\n' + b"--BOUNDARY--\r\n" + ) + with pytest.raises(MultiPartException): + await _run_chunk_parser(body, "BOUNDARY") + + +async def test_chunk_parser_oversized_args_rejected(monkeypatch): + """An oversized bound-args field is rejected instead of buffered unbounded.""" + monkeypatch.setattr( + "reflex_components_core.core._upload.MAX_UPLOAD_EVENT_ARGS_BYTES", 8 + ) + body = _multipart_body("BOUNDARY", args='{"field": "much too long to fit"}') + with pytest.raises(MultiPartException): + await _run_chunk_parser(body, "BOUNDARY") + + def test_styled_upload_create(): styled_up_comp_1 = StyledUpload.create() assert isinstance(styled_up_comp_1, StyledUpload) From 704f67435c0c831de4dd1004128d2c3bf025eb4d Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Thu, 11 Jun 2026 13:45:51 -0700 Subject: [PATCH 6/6] Bump reflex-base min dep to dev version Indicate that reflex-components-core will depend tightly on the next public release of reflex-base --- packages/reflex-components-core/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/reflex-components-core/pyproject.toml b/packages/reflex-components-core/pyproject.toml index 54e72c9625b..a4e8df43e62 100644 --- a/packages/reflex-components-core/pyproject.toml +++ b/packages/reflex-components-core/pyproject.toml @@ -8,7 +8,7 @@ authors = [{ name = "Khaleel Al-Adhami", email = "khaleel@reflex.dev" }] maintainers = [{ name = "Khaleel Al-Adhami", email = "khaleel@reflex.dev" }] requires-python = ">=3.10" dependencies = [ - "reflex-base >= 0.9.5", + "reflex-base >= 0.9.5.post2.dev0", "reflex-components-lucide >= 0.9.0", "reflex-components-sonner >= 0.9.0", "python_multipart >= 0.0.21",