Revert "fix: use is_file_path_valid instead of is_safe_path (#18316…

…) (#18642)" (#18696)

This reverts commit 537966c.

frappe/core/doctype/file/file.py

@@ -16,6 +16,7 @@
 from frappe import _
 from frappe.model.document import Document
 from frappe.utils import call_hook_method, cint, get_files_path, get_hook_method, get_url
+from frappe.utils.file_manager import is_safe_path
 from frappe.utils.image import optimize_image, strip_exif_data
 
 from .exceptions import AttachmentLimitReached, FolderNotEmpty, MaxFileSizeReachedError
@@ -86,9 +87,9 @@ def validate(self):
 			self.handle_is_private_changed()
 
 		if not self.is_folder:
-			# get_full_path validates file URL and name
-			full_path = self.get_full_path()
-			self.validate_file_on_disk(full_path)
+			self.validate_file_path()
+			self.validate_file_url()
+			self.validate_file_on_disk()
 
 		self.file_size = frappe.form_dict.file_size or self.file_size
 
@@ -139,12 +140,16 @@ def get_name_based_on_parent_folder(self) -> str | None:
 	def get_successors(self):
 		return frappe.get_all("File", filters={"folder": self.name}, pluck="name")
 
-	def is_file_path_valid(self, file_path):
-		"""Return True if file path is a valid path for a local file"""
+	def validate_file_path(self):
+		if self.is_remote_file:
+			return
 
 		base_path = os.path.realpath(get_files_path(is_private=self.is_private))
-		if os.path.realpath(file_path).startswith(base_path):
-			return True
+		if not os.path.realpath(self.get_full_path()).startswith(base_path):
+			frappe.throw(
+				_("The File URL you've entered is incorrect"),
+				title=_("Invalid File URL"),
+			)
 
 	def validate_file_url(self):
 		if self.is_remote_file or not self.file_url:
@@ -271,11 +276,9 @@ def set_folder_name(self):
 		elif not self.is_home_folder:
 			self.folder = "Home"
 
-	def validate_file_on_disk(self, full_path=None):
+	def validate_file_on_disk(self):
 		"""Validates existence file"""
-
-		if full_path is None:
-			full_path = self.get_full_path()
+		full_path = self.get_full_path()
 
 		if full_path.startswith(URL_PREFIXES):
 			return True
@@ -455,9 +458,6 @@ def get_full_path(self):
 		if "/files/" in file_path and file_path.startswith(site_url):
 			file_path = file_path.split(site_url, 1)[1]
 
-		if file_path.startswith(URL_PREFIXES):
-			return file_path
-
 		if "/" not in file_path:
 			if self.is_private:
 				file_path = f"/private/files/{file_path}"
@@ -470,10 +470,13 @@ def get_full_path(self):
 		elif file_path.startswith("/files/"):
 			file_path = get_files_path(*file_path.split("/files/", 1)[1].split("/"))
 
+		elif file_path.startswith(URL_PREFIXES):
+			pass
+
 		elif not self.file_url:
 			frappe.throw(_("There is some problem with the file url: {0}").format(file_path))
 
-		if not self.is_file_path_valid(file_path):
+		if not is_safe_path(file_path):
 			frappe.throw(_("Cannot access file path {0}").format(file_path))
 
 		if os.path.sep in self.file_name:

frappe/core/doctype/file/test_file.py

@@ -433,7 +433,6 @@ def test_file_url_validation(self):
 		self.assertRaisesRegex(IOError, "does not exist", test_file.validate)
 
 		test_file.file_url = None
-		test_file.is_private = 1
 		test_file.file_name = "/private/files/_file"
 		self.assertRaisesRegex(ValidationError, "File name cannot have", test_file.validate)
 

frappe/utils/file_manager.py

@@ -449,3 +449,15 @@ def add_attachments(doctype, name, attachments):
 			files.append(f)
 
 	return files
+
+
+def is_safe_path(path: str) -> bool:
+	if path.startswith(("http://", "https://")):
+		return True
+
+	basedir = frappe.get_site_path()
+	# ref: https://docs.python.org/3/library/os.path.html#os.path.commonpath
+	matchpath = os.path.realpath(os.path.abspath(path))
+	basedir = os.path.realpath(os.path.abspath(basedir))
+
+	return basedir == os.path.commonpath((basedir, matchpath))