diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index d9c966cc95..ee2c9987b6 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -1,4 +1,4 @@ -# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE """ @@ -7,7 +7,6 @@ record of files naming for same name files: file.gif, file-1.gif, file-2.gif etc """ -import base64 import hashlib import imghdr import io @@ -17,9 +16,10 @@ import os import re import shutil import zipfile +from typing import TYPE_CHECKING, Tuple import requests -import requests.exceptions +from requests.exceptions import HTTPError, SSLError from PIL import Image, ImageFile, ImageOps from io import BytesIO from urllib.parse import quote, unquote @@ -31,6 +31,11 @@ from frappe.utils import call_hook_method, cint, cstr, encode, get_files_path, g from frappe.utils.image import strip_exif_data, optimize_image from frappe.utils.file_manager import safe_b64decode +if TYPE_CHECKING: + from PIL.ImageFile import ImageFile + from requests.models import Response + + class MaxFileSizeReachedError(frappe.ValidationError): pass @@ -276,7 +281,7 @@ class File(Document): image, filename, extn = get_local_image(self.file_url) else: image, filename, extn = get_web_image(self.file_url) - except (requests.exceptions.HTTPError, requests.exceptions.SSLError, IOError, TypeError): + except (HTTPError, SSLError, IOError, TypeError): return size = width, height @@ -648,9 +653,17 @@ def setup_folder_path(filename, new_parent): from frappe.model.rename_doc import rename_doc rename_doc("File", file.name, file.get_name_based_on_parent_folder(), ignore_permissions=True) -def get_extension(filename, extn, content): +def get_extension(filename, extn, content: bytes = None, response: "Response" = None) -> str: mimetype = None + if response: + content_type = response.headers.get("Content-Type") + + if content_type: + _extn = mimetypes.guess_extension(content_type) + if _extn: + return _extn[1:] + if extn: # remove '?' char and parameters from extn if present if '?' in extn: @@ -693,14 +706,14 @@ def get_local_image(file_url): return image, filename, extn -def get_web_image(file_url): +def get_web_image(file_url: str) -> Tuple["ImageFile", str, str]: # download file_url = frappe.utils.get_url(file_url) r = requests.get(file_url, stream=True) try: r.raise_for_status() - except requests.exceptions.HTTPError as e: - if "404" in e.args[0]: + except HTTPError: + if r.status_code == 404: frappe.msgprint(_("File '{0}' not found").format(file_url)) else: frappe.msgprint(_("Unable to read file format for {0}").format(file_url)) @@ -719,7 +732,10 @@ def get_web_image(file_url): filename = get_random_filename() extn = None - extn = get_extension(filename, extn, r.content) + extn = get_extension(filename, extn, response=r) + if extn == "bin": + extn = get_extension(filename, extn, content=r.content) or "png" + filename = "/files/" + strip(unquote(filename)) return image, filename, extn diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index 3ae5b87128..ba83dfca19 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -1,11 +1,11 @@ -# -*- coding: utf-8 -*- -# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE import base64 import json import frappe import os import unittest + from frappe import _ from frappe.core.doctype.file.file import File, get_attached_images, move_file, get_files_in_folder, unzip_file from frappe.utils import get_files_path @@ -384,6 +384,16 @@ class TestFile(unittest.TestCase): test_file.make_thumbnail() self.assertEquals(test_file.thumbnail_url, '/files/image_small.jpg') + # test web image without extension + test_file = frappe.get_doc({ + "doctype": "File", + "file_name": 'logo', + "file_url": frappe.utils.get_url('/_test/assets/image'), + }).insert(ignore_permissions=True) + + test_file.make_thumbnail() + self.assertTrue(test_file.thumbnail_url.endswith("_small.jpeg")) + # test local image test_file.db_set('thumbnail_url', None) test_file.reload() diff --git a/frappe/tests/test_website.py b/frappe/tests/test_website.py index 4d53fb7ba6..97b9fc9b67 100644 --- a/frappe/tests/test_website.py +++ b/frappe/tests/test_website.py @@ -1,7 +1,9 @@ import unittest +from unittest.mock import patch import frappe from frappe.utils import set_request +from frappe.website.page_renderers.static_page import StaticPage from frappe.website.serve import get_response, get_response_content from frappe.website.utils import (build_response, clear_website_cache, get_home_page) @@ -97,6 +99,19 @@ class TestWebsite(unittest.TestCase): response = get_response() self.assertEqual(response.status_code, 200) + set_request(method="GET", path="/_test/assets/image.jpg") + response = get_response() + self.assertEqual(response.status_code, 200) + + set_request(method="GET", path="/_test/assets/image") + response = get_response() + self.assertEqual(response.status_code, 200) + + with patch.object(StaticPage, "render") as static_render: + set_request(method="GET", path="/_test/assets/image") + response = get_response() + static_render.assert_called() + def test_error_page(self): set_request(method='GET', path='/_test/problematic_page') response = get_response() @@ -127,7 +142,6 @@ class TestWebsite(unittest.TestCase): response = get_response() self.assertEqual(response.status_code, 404) - def test_redirect(self): import frappe.hooks frappe.set_user('Administrator') diff --git a/frappe/website/page_renderers/static_page.py b/frappe/website/page_renderers/static_page.py index 632e9b4302..48bf0f2aec 100644 --- a/frappe/website/page_renderers/static_page.py +++ b/frappe/website/page_renderers/static_page.py @@ -6,6 +6,7 @@ from werkzeug.wsgi import wrap_file import frappe from frappe.website.page_renderers.base_renderer import BaseRenderer +from frappe.website.utils import is_binary_file UNSUPPORTED_STATIC_PAGE_TYPES = ('html', 'md', 'js', 'xml', 'css', 'txt', 'py', 'json') @@ -20,21 +21,20 @@ class StaticPage(BaseRenderer): return for app in frappe.get_installed_apps(): file_path = frappe.get_app_path(app, 'www') + '/' + self.path - if os.path.isfile(file_path): + if os.path.isfile(file_path) and is_binary_file(file_path): self.file_path = file_path def can_render(self): return self.is_valid_file_path() and self.file_path def is_valid_file_path(self): - if ('.' not in self.path): - return False extension = self.path.rsplit('.', 1)[-1] if extension in UNSUPPORTED_STATIC_PAGE_TYPES: return False return True def render(self): + # file descriptor to be left open, closed by middleware f = open(self.file_path, 'rb') response = Response(wrap_file(frappe.local.request.environ, f), direct_passthrough=True) response.mimetype = mimetypes.guess_type(self.file_path)[0] or 'application/octet-stream' diff --git a/frappe/website/page_renderers/template_page.py b/frappe/website/page_renderers/template_page.py index cf017be30b..ff3e8509bd 100644 --- a/frappe/website/page_renderers/template_page.py +++ b/frappe/website/page_renderers/template_page.py @@ -7,7 +7,7 @@ from frappe.website.router import get_page_info from frappe.website.page_renderers.base_template_page import BaseTemplatePage from frappe.website.router import get_base_template from frappe.website.utils import (extract_comment_tag, extract_title, get_next_link, - get_toc, get_frontmatter, cache_html, get_sidebar_items) + get_toc, get_frontmatter, is_binary_file, cache_html, get_sidebar_items) WEBPAGE_PY_MODULE_PROPERTIES = ("base_template_path", "template", "no_cache", "sitemap", "condition_field") @@ -39,7 +39,7 @@ class TemplatePage(BaseTemplatePage): for dirname in folders: search_path = os.path.join(app_path, dirname, self.path) for file_path in self.get_index_path_options(search_path): - if os.path.isfile(file_path): + if os.path.isfile(file_path) and not is_binary_file(file_path): self.app = app self.app_path = app_path self.file_dir = dirname diff --git a/frappe/website/utils.py b/frappe/website/utils.py index 9cf8804dd1..1772d8ada1 100644 --- a/frappe/website/utils.py +++ b/frappe/website/utils.py @@ -1,10 +1,10 @@ -# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE import json import mimetypes import os import re -from functools import wraps +from functools import cache, wraps from typing import Dict, Optional import yaml @@ -511,3 +511,11 @@ def add_preload_headers(response): except Exception: import traceback traceback.print_exc() + +@cache +def is_binary_file(path): + # ref: https://stackoverflow.com/a/7392391/10309266 + textchars = bytearray({7,8,9,10,12,13,27} | set(range(0x20, 0x100)) - {0x7f}) + with open(path, 'rb') as f: + content = f.read(1024) + return bool(content.translate(None, textchars)) diff --git a/frappe/www/_test/assets/image b/frappe/www/_test/assets/image new file mode 100644 index 0000000000..4a2c1552b9 Binary files /dev/null and b/frappe/www/_test/assets/image differ