🐛(backend) strip whitespace from media URLs in CORS proxy#2004
🐛(backend) strip whitespace from media URLs in CORS proxy#2004maboukerfa wants to merge 1 commit intosuitenumerique:mainfrom
Conversation
09d3617 to
94b7f40
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/backend/core/api/viewsets.py`:
- Line 2069: The code calls request.query_params.get("url").strip() which raises
AttributeError if the "url" param is missing; change it to safely handle None by
first retrieving the raw param (e.g., raw_url = request.query_params.get("url"))
and then either use raw_url = raw_url or raw_url = "" if None (or use
request.query_params.get("url", "")) before calling .strip(); ensure the
subsequent logic that checks if not url still returns the expected 400 response.
Update the assignment (replace the problematic url =
request.query_params.get("url").strip() line) in the view handling the docs CORS
proxy so the code normalizes and validates the URL without calling .strip() on
None.
In `@src/backend/core/tests/documents/test_api_documents_cors_proxy.py`:
- Around line 73-77: The registered responses mock uses the padded URL string so
it doesn't match the actual request after the view strips whitespace; change the
mock registration to use the stripped URL (e.g., assign stripped_url =
url_to_fetch.strip() and call responses.get(stripped_url,...)) while still
passing the original padded url_to_fetch into client.get's querystring, and add
a second test that calls the endpoint with ?url= omitted or blank and asserts a
400 response to prevent the None.strip() regression referenced in viewsets.py
(around the strip call).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e91efa4c-b2ab-4a36-8d60-49a3bf2686d9
📒 Files selected for processing (2)
src/backend/core/api/viewsets.pysrc/backend/core/tests/documents/test_api_documents_cors_proxy.py
When exporting a document to PDF, having whitespace before or after the media URL causes the image to not be downloaded via the CORS proxy, resulting in missing images in the exported PDF. Signed-off-by: Mohamed El Amine BOUKERFA <boukerfa.ma@gmail.com>
94b7f40 to
c67000c
Compare
|
@lunika, can you please re-run the tests actions, thanx |
Purpose
Fixes an issue where media URLs with leading or trailing whitespace fail to download via the CORS proxy during PDF export, resulting in missing images in the exported PDF.
Screen.Recording.2026-03-15.at.16.18.45.mov
Proposal
Added
.strip()to the URL query parameter in the CORS proxy endpoint to handle URLs with accidental leading/trailing whitespace.