From 5d9b48a2c51ba3edf1965dea19c5537363f82c24 Mon Sep 17 00:00:00 2001
From: Dirk van der Laarse <dirk@laarse.co.za>
Date: Wed, 19 Aug 2026 08:59:14 +0000
Subject: [PATCH] fix: check urls so that pasting into comments work

---
 cloud_storage/cloud_storage/overrides/file.py | 22 +++++++++
 cloud_storage/tests/test_file.py              | 49 +++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/cloud_storage/cloud_storage/overrides/file.py b/cloud_storage/cloud_storage/overrides/file.py
index 94c9c74..8179e05 100644
--- a/cloud_storage/cloud_storage/overrides/file.py
+++ b/cloud_storage/cloud_storage/overrides/file.py
@@ -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
diff --git a/cloud_storage/tests/test_file.py b/cloud_storage/tests/test_file.py
index 5b779f4..f72cfb4 100644
--- a/cloud_storage/tests/test_file.py
+++ b/cloud_storage/tests/test_file.py
@@ -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"