Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions cloud_storage/cloud_storage/overrides/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,28 @@ def is_remote_file(self) -> bool:
return self.file_url.startswith(URL_PREFIXES) # type: ignore
return not self.content

@property
def unique_url(self) -> str:
"""
HASH: 69a495579a729909f4df7a45855165eee4a208f4
REPO: https://github.com/frappe/frappe
PATH: frappe/core/doctype/file/file.py
METHOD: unique_url

Cloud storage file URLs already carry a query string
(/api/method/retrieve?key=...), so the fid parameter has to be appended
with "&". Core assumes a plain path and always uses "?", which corrupts
the key parameter and makes retrieve() return a 404.
"""
from urllib.parse import urlencode

file_url = self.file_url or ""
if not self.is_private:
return file_url

separator = "&" if "?" in file_url else "?"
return file_url + separator + urlencode({"fid": self.name})

def validate(self) -> None:
"""
HASH: 69a495579a729909f4df7a45855165eee4a208f4
Expand Down
49 changes: 49 additions & 0 deletions cloud_storage/tests/test_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,3 +353,52 @@ def test_migration_command(mocked_s3_client, example_file_record_6):
assert (
s3_file_size == original_file_size
), f"File size mismatch: local={original_file_size}, s3={s3_file_size}"


def test_unique_url_appends_fid_to_existing_query_string():
"""Private cloud storage URLs already have a query string, fid must join with "&".

Core File.unique_url always uses "?", which produces
/api/method/retrieve?key=folder/x.png?fid=<name> and makes retrieve() read
"folder/x.png?fid=<name>" as the s3_key, throwing DoesNotExistError. This is
the path pasted images in comments and emails take via
frappe.core.doctype.file.utils.extract_images_from_html.
"""
frappe.set_user("Administrator")

private_file = frappe.get_doc(
{
"doctype": "File",
"file_name": "pasted.png",
"file_url": "/api/method/retrieve?key=test_folder/pasted.png",
"is_private": 1,
}
)
private_file.name = "test-unique-url-private"
assert (
private_file.unique_url
== "/api/method/retrieve?key=test_folder/pasted.png&fid=test-unique-url-private"
)

public_file = frappe.get_doc(
{
"doctype": "File",
"file_name": "pasted.png",
"file_url": "/api/method/retrieve?key=test_folder/pasted.png",
"is_private": 0,
}
)
public_file.name = "test-unique-url-public"
assert public_file.unique_url == "/api/method/retrieve?key=test_folder/pasted.png"

# files stored locally keep the core behavior
local_file = frappe.get_doc(
{
"doctype": "File",
"file_name": "pasted.png",
"file_url": "/private/files/pasted.png",
"is_private": 1,
}
)
local_file.name = "test-unique-url-local"
assert local_file.unique_url == "/private/files/pasted.png?fid=test-unique-url-local"
Loading