Making rx.download() more robust
#5954
Conversation
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR enhances the rx.download() function to support blob-based downloads, addressing encoding issues encountered with binary data formats like PEM and DER files. The changes add two new parameters: as_blob (boolean) and mime_type (string, defaults to "application/octet-stream"). When as_blob=True, the implementation uses a "DOWNLOAD_AS_BLOB:" prefix pattern similar to the existing upload mechanism to signal the frontend to use JavaScript's Blob API and URL.createObjectURL() instead of direct URL assignment. This prevents data corruption that occurs when binary content passes through urllib.parse.quote(data) in the standard download path. The modification extends the existing download functionality while maintaining full backward compatibility - existing code continues to work unchanged, and the new blob approach is only activated when explicitly requested.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
| reflex/event.py | 4/5 | Added as_blob and mime_type parameters to download function with proper validation and blob prefix logic |
| reflex/.templates/web/utils/state.js | 4/5 | Enhanced download event handler to detect blob prefix and create blob URLs using JavaScript Blob API |
Confidence score: 4/5
- This PR is safe to merge with minimal risk as it maintains backward compatibility and addresses a genuine technical limitation
- Score reflects well-structured implementation following existing patterns, but lacks comprehensive testing coverage for the new functionality
- Pay close attention to reflex/event.py for parameter validation logic and edge cases around the blob/URL mutual exclusivity
Sequence Diagram
sequenceDiagram
participant User
participant Frontend
participant EventHandler
participant StateJS
participant Browser
User->>Frontend: "Trigger download action"
Frontend->>EventHandler: "Call rx.download() with as_blob=True"
EventHandler->>EventHandler: "Check parameters and create download EventSpec"
alt as_blob is True
EventHandler->>EventHandler: "Prepend 'DOWNLOAD_AS_BLOB:' to data"
else as_blob is False or None
EventHandler->>EventHandler: "Use existing logic (data URI or URL encoding)"
end
EventHandler->>StateJS: "Emit '_download' event with payload"
StateJS->>StateJS: "Process event in applyEvent function"
alt URL starts with 'DOWNLOAD_AS_BLOB:'
StateJS->>StateJS: "Extract data after 'DOWNLOAD_AS_BLOB:' prefix"
StateJS->>Browser: "Create Blob with extracted data and mime type"
StateJS->>Browser: "Generate object URL from blob"
StateJS->>Browser: "Set anchor href to blob URL"
else Regular URL or data URI
StateJS->>Browser: "Set anchor href to provided URL"
alt URL contains 'getBackendURL(env.UPLOAD)'
StateJS->>StateJS: "Evaluate and replace with actual backend URL"
end
end
StateJS->>Browser: "Set download filename on anchor element"
StateJS->>Browser: "Trigger click on hidden anchor element"
Browser->>User: "Download file to user's device"
StateJS->>StateJS: "Remove anchor element from DOM"
2 files reviewed, 1 comment
CodSpeed Performance ReportMerging #5954 will not alter performanceComparing Summary
|
|
closing in favor of #5957, thanks for the help! |
|
@riebecj we ended up taking a more generic fix, but I'm curious if you had a repro case to share where you were hitting issues. I tried to reproduce this and wasn't able to. """Test files generated with:
ssh-keygen -t rsa -m PEM -f ./foo.pem
openssl rsa -outform der -in ./foo.pem -out foo.der
"""
from pathlib import Path
import reflex as rx
class State(rx.State):
@rx.event
def download_pem(self):
return rx.download(
data=Path("foo.pem").read_text(),
filename="certificate.pem",
)
@rx.event
def download_der(self):
return rx.download(
data=Path("foo.der").read_bytes(),
filename="certificate.der",
)
def index() -> rx.Component:
return rx.fragment(
rx.color_mode.button(),
rx.vstack(
rx.link("Download PEM file", on_click=State.download_pem),
rx.link("Download DER file", on_click=State.download_der),
),
)
app = rx.App()
app.add_page(index) |
Instead of reading a file directly, pass is as an rx.download(
data=rx.Var.create(Path("foo.pem").read_text()),
filename="certificate.pem",
)You'll see what I mean. |
|
@masenf @adhami3310 See my comment |
Ran into issues with formatting when downloading files with PEM and DER formats due to the data passing through
urllib.parse.quote(data). So, I followed a similar approach togetBackendURL(env.UPLOAD)and add aDOWNLOAD_AS_BLOB:prepend to the data which is checked for and processed. Ifas_blobis set to True, it is the first thing processed, otherwise the function logic continues as previously designed.Adds two new kwargs to
rx.download():-
as_blob: tells the JS event to useconst blob = new Blob(...)andURL.createObjectURL(blob)to download the object.-
mime_type: Optional mime-type specification when downloadingas_blob. The default is"application/octet-stream"as JS blobs are typically byte data, but the user can get more specific (e.g.application/x-pem-file).Also, there are no tests for
rx.download()so no new tests were added.All Submissions:
Type of change
Please delete options that are not relevant.
New Feature Submission:
Changes To Core Features: