Skip to content

fix(upload): handle unknown event handlers - #6861

Open
hariomlohardev wants to merge 1 commit into
reflex-dev:mainfrom
hariomlohardev:fix/6860-upload-unknown-handler
Open

fix(upload): handle unknown event handlers#6861
hariomlohardev wants to merge 1 commit into
reflex-dev:mainfrom
hariomlohardev:fix/6860-upload-unknown-handler

Conversation

@hariomlohardev

@hariomlohardev hariomlohardev commented Aug 8, 2026

Copy link
Copy Markdown

All Submissions:

Type of change

  • Bug fix (non-breaking change which fixes an existing issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

New Feature Submission:

Not applicable — this is a bug fix.

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Description

Fixes #6860

The upload endpoint previously performed a direct registry lookup for the Reflex-Event-Handler header. If the header referenced an unknown or stale event handler, the lookup raised an unhandled KeyError and resulted in a 500 response.

This change:

  • Uses a guarded registry lookup with .get().
  • Returns a controlled 400 Bad Request for an unknown upload event handler.
  • Adds a focused unit test covering the unknown-handler case.
  • Ensures the invalid handler is rejected before form parsing or event dispatch.

Valid registered upload handlers continue to follow the existing behavior.

Review in cubic

Signed-off-by: hariomloharapps <hariomlohar.new@gmail.com>
@hariomlohardev
hariomlohardev requested a review from a team as a code owner August 8, 2026 19:13
@greptile-apps

greptile-apps Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR attempts to return a controlled 400 response when an upload request references an unregistered event handler.

  • Replaces direct registry subscription with a guarded lookup.
  • Adds an unknown-handler error response and focused regression test.
  • The new lookup currently wraps the handler name in an unhashable list, breaking both valid and invalid upload requests.

Confidence Score: 4/5

This PR is not safe to merge until the registry lookup uses the handler-name string rather than an unhashable list.

The changed lookup raises TypeError for every handler name, preventing valid upload dispatch and the intended controlled rejection of unknown handlers.

Files Needing Attention: packages/reflex-components-core/src/reflex_components_core/core/_upload.py

Important Files Changed

Filename Overview
packages/reflex-components-core/src/reflex_components_core/core/_upload.py Adds unknown-handler validation, but the list passed to dict.get raises TypeError before the validation can run.
tests/units/test_app.py Adds focused coverage for the intended 400 response; the test should expose the malformed registry lookup when run.

Reviews (1): Last reviewed commit: "fix(upload): handle unknown event handle..." | Re-trigger Greptile

Comment on lines +809 to +811
registered_event_handler = RegistrationContext.get().event_handlers.get([
handler_name
]
])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 List key breaks handler lookup

When any upload request supplies the required handler header, passing [handler_name] to the plain dictionary's get method raises TypeError: unhashable type: 'list', causing valid uploads and the intended unknown-handler rejection to fail before the new guard runs.

Suggested change
registered_event_handler = RegistrationContext.get().event_handlers.get([
handler_name
]
])
registered_event_handler = RegistrationContext.get().event_handlers.get(
handler_name
)

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/reflex-components-core/src/reflex_components_core/core/_upload.py">

<violation number="1" location="packages/reflex-components-core/src/reflex_components_core/core/_upload.py:809">
P1: Upload requests now fail with `TypeError: unhashable type: 'list'` before handler resolution, including registered handlers. Pass `handler_name` directly to `.get()` so unknown handlers reach the intended 400 response.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment on lines +809 to +811
registered_event_handler = RegistrationContext.get().event_handlers.get([
handler_name
]
])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: Upload requests now fail with TypeError: unhashable type: 'list' before handler resolution, including registered handlers. Pass handler_name directly to .get() so unknown handlers reach the intended 400 response.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-components-core/src/reflex_components_core/core/_upload.py, line 809:

<comment>Upload requests now fail with `TypeError: unhashable type: 'list'` before handler resolution, including registered handlers. Pass `handler_name` directly to `.get()` so unknown handlers reach the intended 400 response.</comment>

<file context>
@@ -805,9 +806,14 @@ async def upload_file(request: Request):
 
         token, handler_name = _require_upload_headers(request)
-        registered_event_handler = RegistrationContext.get().event_handlers[
+        registered_event_handler = RegistrationContext.get().event_handlers.get([
             handler_name
-        ]
</file context>
Suggested change
registered_event_handler = RegistrationContext.get().event_handlers.get([
handler_name
]
])
registered_event_handler = RegistrationContext.get().event_handlers.get(
handler_name
)

@harsh21234i

harsh21234i commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Thanks for the focused fix and regression test. I think there is one blocker before this can merge.

packages/reflex-components-core/src/reflex_components_core/core/_upload.py currently does:

registered_event_handler = RegistrationContext.get().event_handlers.get([
handler_name
])

Because [handler_name] is a list, this raises TypeError: cannot use 'list' as a dict key for both unknown handlers and
valid registered upload handlers. I confirmed it by running:

uv run --frozen pytest tests/units/test_app.py::test_upload_file_unknown_handler_returns_400 tests/units/
test_app.py::test_upload_file -q

Result: 4 failed, all from that lookup.

This should be:

registered_event_handler = RegistrationContext.get().event_handlers.get(
handler_name
)

Also, since this is a user-facing upload bugfix for #6860, towncrier check currently fails because there is no news
fragment. Please add a bugfix fragment, likely packages/reflex-components-core/news/6860.bugfix.md or
news/6860.bugfix.md, depending maintainer preference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upload endpoint returns 500 for unknown Reflex-Event-Handler header

2 participants