Use configured base_url for signed upload/download URLs#100
Draft
mrjefftang wants to merge 1 commit into
Draft
Conversation
The signed upload and download URLs were built from request.scheme and request.host, which ignored the configured SUPERNOTE_BASE_URL. Build them from config.base_url instead so the application's base URL is honored (falling back to the configured host:port when unset). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the Supernote server’s signed upload/download URL generation so that externally visible URLs honor the configured SUPERNOTE_BASE_URL (via config.base_url) instead of being derived from the incoming request’s scheme/host.
Changes:
- Switched signed upload URL construction to use
config.base_urlin summary, web, and device upload-apply endpoints. - Switched signed download URL construction to use
config.base_urlin summary and device download/convert endpoints.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| supernote/server/routes/summary.py | Uses config.base_url for signed upload/apply-summary and download-summary URLs. |
| supernote/server/routes/file_web.py | Uses config.base_url for web upload/apply signed URL generation. |
| supernote/server/routes/file_device.py | Uses config.base_url for device upload/apply, download/apply, and note conversion download URLs. |
Comment on lines
+336
to
+340
| config = request.app["config"] | ||
| encoded_name = urllib.parse.quote(summary.handwrite_inner_name) | ||
| download_path = f"/api/oss/download?path={encoded_name}" | ||
| signed_path = await url_signer.sign(download_path, user=user_email) | ||
| download_url = f"{request.scheme}://{request.host}{signed_path}" | ||
| download_url = f"{config.base_url}{signed_path}" |
Comment on lines
559
to
+561
| path_to_sign = f"/api/oss/upload?path={encoded_name}" | ||
| signed_path = await url_signer.sign(path_to_sign, user=request["user"]) | ||
| full_url = f"{request.scheme}://{request.host}{signed_path}" | ||
| full_url = f"{config.base_url}{signed_path}" |
Comment on lines
293
to
297
| simple_path = f"/api/oss/upload?path={encoded_name}" | ||
| full_upload_url_path = await url_signer.sign(simple_path, user=request["user"]) | ||
| full_upload_url = f"{request.scheme}://{request.host}{full_upload_url_path}" | ||
| full_upload_url = f"{config.base_url}{full_upload_url_path}" | ||
|
|
||
| # Extract signature and timestamp using UrlSigner helpers |
Comment on lines
397
to
+401
| path_to_sign = f"/api/oss/download?path={info.id}" | ||
|
|
||
| # helper returns: ...?signature=... | ||
| signed_path = await url_signer.sign(path_to_sign, user=user_email) | ||
| download_url = f"{request.scheme}://{request.host}{signed_path}" | ||
| download_url = f"{config.base_url}{signed_path}" |
Comment on lines
559
to
563
| path_to_sign = f"/api/oss/download?path={res.storage_key}" | ||
| signed_path = await url_signer.sign(path_to_sign, user=user_email) | ||
| download_url = f"{request.scheme}://{request.host}{signed_path}" | ||
| download_url = f"{config.base_url}{signed_path}" | ||
|
|
||
| png_pages.append(PngPageVO(page_no=res.page_no, url=download_url)) |
Comment on lines
588
to
592
| # Generate signed URL for PDF | ||
| path_to_sign = f"/api/oss/download?path={storage_key}" | ||
| signed_path = await url_signer.sign(path_to_sign, user=user_email) | ||
| download_url = f"{request.scheme}://{request.host}{signed_path}" | ||
| download_url = f"{config.base_url}{signed_path}" | ||
|
|
Comment on lines
+297
to
+301
| full_url = f"{config.base_url}{full_url_path}" | ||
|
|
||
| part_path = f"/api/oss/upload/part?path={encoded_name}" | ||
| part_url_path = await url_signer.sign(part_path, user=user_email) | ||
| part_url = f"{request.scheme}://{request.host}{part_url_path}" | ||
| part_url = f"{config.base_url}{part_url_path}" |
allenporter
requested changes
Jul 13, 2026
allenporter
left a comment
allenporter
left a comment
Owner
There was a problem hiding this comment.
Hi,
I agree this is a valid issue to fix, thank you for the contribution. co-piilot is calling out that of the base url is not set this may not perform properly (e.g. it will now always return localhost when the environment variable is not set). What's your take on this?
Generally please reply / ack / respond to the co-pilot comments and you can mark ready for review when ready.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The signed upload and download URLs were built from request.scheme and request.host, which ignored the configured SUPERNOTE_BASE_URL.
Build them from config.base_url instead so the application's base URL is honored (falling back to the configured host:port when unset).