diff --git a/mypy.ini b/mypy.ini index 3813858dd5..750c6473bc 100644 --- a/mypy.ini +++ b/mypy.ini @@ -407,3 +407,6 @@ strict_optional = False [mypy-tools.lib.html_branches] strict_optional = False + +[mypy-zthumbor.loaders.helpers] +strict_optional = False diff --git a/static/js/lightbox.js b/static/js/lightbox.js index 2bc90cb819..8989b8f006 100644 --- a/static/js/lightbox.js +++ b/static/js/lightbox.js @@ -114,7 +114,12 @@ exports.open = function (image, options) { $source = $parent.attr("data-id"); } else { $type = "image"; - $source = $image.attr("src"); + // thumbor supplies the src as thumbnail, data-original as full-sized. + if ($image.attr("data-original")) { + $source = $image.attr("data-original"); + } else { + $source = $image.attr("src"); + } } var message = message_store.get($message.attr("zid")); if (message === undefined) { diff --git a/zerver/lib/bugdown/__init__.py b/zerver/lib/bugdown/__init__.py index 1a0351c9b0..ef7de8d309 100644 --- a/zerver/lib/bugdown/__init__.py +++ b/zerver/lib/bugdown/__init__.py @@ -36,6 +36,7 @@ from zerver.lib.emoji import translate_emoticons, emoticon_regex from zerver.lib.mention import possible_mentions, \ possible_user_group_mentions, extract_user_group from zerver.lib.notifications import encode_stream +from zerver.lib.thumbnail import is_thumbor_enabled from zerver.lib.timeout import timeout, TimeoutExpired from zerver.lib.cache import cache_with_key, NotFoundInCache from zerver.lib.url_preview import preview as link_preview @@ -202,7 +203,8 @@ def add_a( desc: Optional[str]=None, class_attr: str="message_inline_image", data_id: Optional[str]=None, - insertion_index: Optional[int]=None + insertion_index: Optional[int]=None, + use_thumbnails: Optional[bool]=True ) -> None: title = title if title is not None else url_filename(link) title = title if title else "" @@ -222,7 +224,21 @@ def add_a( if data_id is not None: a.set("data-id", data_id) img = markdown.util.etree.SubElement(a, "img") - img.set("src", url) + if is_thumbor_enabled() and use_thumbnails: + # We strip leading '/' from relative URLs here to ensure + # consistency in what gets passed to /thumbnail + url = url.lstrip('/') + img.set("src", "/thumbnail?url={0}&size=thumbnail".format( + urllib.parse.quote(url, safe='') + )) + img.set('data-original', "/thumbnail?url={0}&size=original".format( + urllib.parse.quote(url, safe='') + )) + else: + # TODO: We might want to rename use_thumbnails to + # !already_thumbnailed for clarity. + img.set("src", url) + if class_attr == "message_inline_ref": summary_div = markdown.util.etree.SubElement(div, "div") title_div = markdown.util.etree.SubElement(summary_div, "div") @@ -834,7 +850,8 @@ class InlineInterestingLinkProcessor(markdown.treeprocessors.Treeprocessor): add_a(root, dropbox_image['image'], url, title=dropbox_image.get('title', ""), desc=dropbox_image.get('desc', ""), - class_attr=class_attr) + class_attr=class_attr, + use_thumbnails=False) continue if self.is_image(url): self.handle_image_inlining(root, found_url) @@ -855,7 +872,9 @@ class InlineInterestingLinkProcessor(markdown.treeprocessors.Treeprocessor): youtube = self.youtube_image(url) if youtube is not None: yt_id = self.youtube_id(url) - add_a(root, youtube, url, None, None, "youtube-video message_inline_image", yt_id) + add_a(root, youtube, url, None, None, + "youtube-video message_inline_image", + yt_id, use_thumbnails=False) continue if arguments.db_data and arguments.db_data['sent_by_bot']: @@ -876,7 +895,8 @@ class InlineInterestingLinkProcessor(markdown.treeprocessors.Treeprocessor): vimeo_title = self.vimeo_title(extracted_data) if vimeo_image is not None: add_a(root, vimeo_image, url, vimeo_title, - None, "vimeo-video message_inline_image", vm_id) + None, "vimeo-video message_inline_image", vm_id, + use_thumbnails=False) if vimeo_title is not None: found_url.family.child.text = vimeo_title else: diff --git a/zerver/lib/thumbnail.py b/zerver/lib/thumbnail.py new file mode 100644 index 0000000000..c83fc2919b --- /dev/null +++ b/zerver/lib/thumbnail.py @@ -0,0 +1,69 @@ +# -*- coding: utf-8 -*- +import base64 +import os +import sys +import urllib +from django.conf import settings +from libthumbor import CryptoURL + +ZULIP_PATH = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath('__file__')))) +sys.path.append(ZULIP_PATH) + +from zthumbor.loaders.helpers import ( + THUMBOR_S3_TYPE, THUMBOR_LOCAL_FILE_TYPE, THUMBOR_EXTERNAL_TYPE +) +from zerver.lib.camo import get_camo_url + +def is_thumbor_enabled() -> bool: + return settings.THUMBOR_URL != '' + +def get_source_type(url: str) -> str: + if not url.startswith('/user_uploads/'): + return THUMBOR_EXTERNAL_TYPE + + local_uploads_dir = settings.LOCAL_UPLOADS_DIR + if local_uploads_dir: + return THUMBOR_LOCAL_FILE_TYPE + return THUMBOR_S3_TYPE + +def generate_thumbnail_url(path: str, size: str='0x0') -> str: + if not (path.startswith('https://') or path.startswith('http://')): + path = '/' + path + + if not is_thumbor_enabled(): + if path.startswith('http://'): + return get_camo_url(path) + return path + + # Ignore thumbnailing for static resources. + if path.startswith('/static/'): + return path + + source_type = get_source_type(path) + if source_type == THUMBOR_EXTERNAL_TYPE: + url = path + else: + url = path[len('/user_uploads/'):] + + safe_url = base64.urlsafe_b64encode(url.encode()).decode('utf-8') + image_url = '%s/source_type/%s' % (safe_url, source_type) + width, height = map(int, size.split('x')) + crypto = CryptoURL(key=settings.THUMBOR_KEY) + encrypted_url = crypto.generate( + width=width, + height=height, + smart=True, + filters=['no_upscale()'], + image_url=image_url + ) + + if settings.THUMBOR_URL == 'http://127.0.0.1:9995': + # If THUMBOR_URL is the default then thumbor is hosted on same machine + # as the Zulip server and we should serve a relative URL. + # We add a /thumbor in front of the relative url because we make + # use of a proxy pass to redirect request internally in Nginx to 9995 + # port where thumbor is running. + thumbnail_url = '/thumbor' + encrypted_url + else: + thumbnail_url = urllib.parse.urljoin(settings.THUMBOR_URL, encrypted_url) + return thumbnail_url diff --git a/zerver/tests/fixtures/markdown_test_cases.json b/zerver/tests/fixtures/markdown_test_cases.json index cc4171cfab..73fcba20b8 100644 --- a/zerver/tests/fixtures/markdown_test_cases.json +++ b/zerver/tests/fixtures/markdown_test_cases.json @@ -276,41 +276,41 @@ { "name": "inline_image", "input": "Google logo today: https://www.google.com/images/srpr/logo4w.png\nKinda boring", - "expected_output": "

Google logo today: https://www.google.com/images/srpr/logo4w.png
\nKinda boring

\n
", + "expected_output": "

Google logo today: https://www.google.com/images/srpr/logo4w.png
\nKinda boring

\n
", "backend_only_rendering": true, "text_content": "Google logo today: https:\/\/www.google.com\/images\/srpr\/logo4w.png\nKinda boring\n" }, { "name": "blockquote_inline_image", "input": ">Google logo today:\n>https://www.google.com/images/srpr/logo4w.png\n>Kinda boring", - "expected_output": "
\n

Google logo today:
\nhttps://www.google.com/images/srpr/logo4w.png
\nKinda boring

\n
", + "expected_output": "
\n

Google logo today:
\nhttps://www.google.com/images/srpr/logo4w.png
\nKinda boring

\n
", "backend_only_rendering": true, "text_content": "> Google logo today:\n> https:\/\/www.google.com\/images\/srpr\/logo4w.png\n> Kinda boring\n" }, { "name": "two_inline_images", "input": "Google logo today: https://www.google.com/images/srpr/logo4w.png\nKinda boringGoogle logo today: https://www.google.com/images/srpr/logo4w.png\nKinda boring", - "expected_output": "

Google logo today: https://www.google.com/images/srpr/logo4w.png
\nKinda boringGoogle logo today: https://www.google.com/images/srpr/logo4w.png
\nKinda boring

\n
", + "expected_output": "

Google logo today: https://www.google.com/images/srpr/logo4w.png
\nKinda boringGoogle logo today: https://www.google.com/images/srpr/logo4w.png
\nKinda boring

\n
", "backend_only_rendering": true, "text_content": "Google logo today: https:\/\/www.google.com\/images\/srpr\/logo4w.png\nKinda boringGoogle logo today: https:\/\/www.google.com\/images\/srpr\/logo4w.png\nKinda boring\n" }, { "name": "bulleted_list_inlining", "input": "* Google?\n* Google. https://www.google.com/images/srpr/logo4w.png\n* Google!", - "expected_output": "", + "expected_output": "", "backend_only_rendering": true, "text_content": "\nGoogle?\nGoogle. https://www.google.com/images/srpr/logo4w.png\nGoogle!\n" }, { "name": "only_inline_image", "input": "https://www.google.com/images/srpr/logo4w.png", - "expected_output": "
", + "expected_output": "
", "backend_only_rendering": true }, { "name": "only_named_inline_image", "input": "[Google Link](https://www.google.com/images/srpr/logo4w.png)", - "expected_output": "

Google Link

\n
", + "expected_output": "

Google Link

\n
", "backend_only_rendering": true, "text_content": "Google Link\n" }, @@ -319,12 +319,6 @@ "input": "https://github.com", "expected_output": "

https://github.com

" }, - { - "name": "camo", - "input": "Google logo today: http://www.google.com/images/srpr/logo4w.png", - "expected_output": "

Google logo today: http://www.google.com/images/srpr/logo4w.png

\n
", - "backend_only_rendering": true - }, { "name": "nl2br", "input": "test\nbar", diff --git a/zerver/tests/test_bugdown.py b/zerver/tests/test_bugdown.py index 906b04bdf5..6fa6722e26 100644 --- a/zerver/tests/test_bugdown.py +++ b/zerver/tests/test_bugdown.py @@ -340,10 +340,35 @@ class BugdownTest(ZulipTestCase): self.assertEqual(converted, '

https://vimeo.com/246979354

') @override_settings(INLINE_IMAGE_PREVIEW=True) - def test_inline_image_preview(self) -> None: - with_preview = '

Test: http://cdn.wallpapersafari.com/13/6/16eVjx.jpeg

\n
' - without_preview = '

Test: http://cdn.wallpapersafari.com/13/6/16eVjx.jpeg

' - content = 'Test: http://cdn.wallpapersafari.com/13/6/16eVjx.jpeg' + def test_inline_image_thumbnail_url(self): + # type: () -> None + msg = '[foobar](/user_uploads/2/50/w2G6ok9kr8AMCQCTNAUOFMln/IMG_0677.JPG)' + thumbnail_img = '<' + converted = bugdown_convert(msg) + self.assertIn(thumbnail_img, converted) + + msg = 'https://www.google.com/images/srpr/logo4w.png' + thumbnail_img = '' + converted = bugdown_convert(msg) + self.assertIn(thumbnail_img, converted) + + msg = 'www.google.com/images/srpr/logo4w.png' + thumbnail_img = '' + converted = bugdown_convert(msg) + self.assertIn(thumbnail_img, converted) + + msg = 'https://www.google.com/images/srpr/logo4w.png' + thumbnail_img = '
' + with self.settings(THUMBOR_URL=''): + converted = bugdown_convert(msg) + self.assertIn(thumbnail_img, converted) + + @override_settings(INLINE_IMAGE_PREVIEW=True) + def test_inline_image_preview(self): + # type: () -> None + with_preview = '
' + without_preview = '

http://cdn.wallpapersafari.com/13/6/16eVjx.jpeg

' + content = 'http://cdn.wallpapersafari.com/13/6/16eVjx.jpeg' sender_user_profile = self.example_user('othello') msg = Message(sender=sender_user_profile, sending_client=get_client("test")) @@ -362,7 +387,7 @@ class BugdownTest(ZulipTestCase): @override_settings(INLINE_IMAGE_PREVIEW=True) def test_inline_image_preview_order(self) -> None: content = 'http://imaging.nikon.com/lineup/dslr/df/img/sample/img_01.jpg\nhttp://imaging.nikon.com/lineup/dslr/df/img/sample/img_02.jpg\nhttp://imaging.nikon.com/lineup/dslr/df/img/sample/img_03.jpg' - expected = '

http://imaging.nikon.com/lineup/dslr/df/img/sample/img_01.jpg
\nhttp://imaging.nikon.com/lineup/dslr/df/img/sample/img_02.jpg
\nhttp://imaging.nikon.com/lineup/dslr/df/img/sample/img_03.jpg

\n
' + expected = '

http://imaging.nikon.com/lineup/dslr/df/img/sample/img_01.jpg
\nhttp://imaging.nikon.com/lineup/dslr/df/img/sample/img_02.jpg
\nhttp://imaging.nikon.com/lineup/dslr/df/img/sample/img_03.jpg

\n
' sender_user_profile = self.example_user('othello') msg = Message(sender=sender_user_profile, sending_client=get_client("test")) @@ -370,7 +395,7 @@ class BugdownTest(ZulipTestCase): self.assertEqual(converted, expected) content = 'http://imaging.nikon.com/lineup/dslr/df/img/sample/img_01.jpg\n\n>http://imaging.nikon.com/lineup/dslr/df/img/sample/img_02.jpg\n\n* http://imaging.nikon.com/lineup/dslr/df/img/sample/img_03.jpg\n* https://www.google.com/images/srpr/logo4w.png' - expected = '
\n
\n' + expected = '
\n
\n' sender_user_profile = self.example_user('othello') msg = Message(sender=sender_user_profile, sending_client=get_client("test")) @@ -378,7 +403,7 @@ class BugdownTest(ZulipTestCase): self.assertEqual(converted, expected) content = 'Test 1\n[21136101110_1dde1c1a7e_o.jpg](/user_uploads/1/6d/F1PX6u16JA2P-nK45PyxHIYZ/21136101110_1dde1c1a7e_o.jpg) \n\nNext Image\n[IMG_20161116_023910.jpg](/user_uploads/1/69/sh7L06e7uH7NaX6d5WFfVYQp/IMG_20161116_023910.jpg) \n\nAnother Screenshot\n[Screenshot-from-2016-06-01-16-22-42.png](/user_uploads/1/70/_aZmIEWaN1iUaxwkDjkO7bpj/Screenshot-from-2016-06-01-16-22-42.png)' - expected = '

Test 1
\n21136101110_1dde1c1a7e_o.jpg

\n

Next Image
\nIMG_20161116_023910.jpg

\n

Another Screenshot
\nScreenshot-from-2016-06-01-16-22-42.png

\n
' + expected = '

Test 1
\n21136101110_1dde1c1a7e_o.jpg

\n

Next Image
\nIMG_20161116_023910.jpg

\n

Another Screenshot
\nScreenshot-from-2016-06-01-16-22-42.png

\n
' msg = Message(sender=sender_user_profile, sending_client=get_client("test")) converted = render_markdown(msg, content) @@ -449,7 +474,7 @@ class BugdownTest(ZulipTestCase): with mock.patch('zerver.lib.bugdown.fetch_open_graph_image', return_value=None): converted = bugdown_convert(msg) - self.assertEqual(converted, '

Look at the new dropbox logo: https://www.dropbox.com/static/images/home_logo.png

\n
') + self.assertEqual(converted, '

Look at the new dropbox logo: https://www.dropbox.com/static/images/home_logo.png

\n
') def test_inline_dropbox_bad(self) -> None: # Don't fail on bad dropbox links @@ -463,12 +488,12 @@ class BugdownTest(ZulipTestCase): msg = 'Test: https://github.com/zulip/zulip/blob/master/static/images/logo/zulip-icon-128x128.png' converted = bugdown_convert(msg) - self.assertEqual(converted, '

Test: https://github.com/zulip/zulip/blob/master/static/images/logo/zulip-icon-128x128.png

\n
') + self.assertEqual(converted, '

Test: https://github.com/zulip/zulip/blob/master/static/images/logo/zulip-icon-128x128.png

\n
') msg = 'Test: https://developer.github.com/assets/images/hero-circuit-bg.png' converted = bugdown_convert(msg) - self.assertEqual(converted, '

Test: https://developer.github.com/assets/images/hero-circuit-bg.png

\n
') + self.assertEqual(converted, '

Test: https://developer.github.com/assets/images/hero-circuit-bg.png

\n
') def test_twitter_id_extraction(self) -> None: self.assertEqual(bugdown.get_tweet_id('http://twitter.com/#!/VizzQuotes/status/409030735191097344'), '409030735191097344') @@ -1120,7 +1145,7 @@ class BugdownTest(ZulipTestCase): '

\n' '
' '' - '' + '' '' '
' ) diff --git a/zerver/tests/test_thumbnail.py b/zerver/tests/test_thumbnail.py new file mode 100644 index 0000000000..8c7a60b884 --- /dev/null +++ b/zerver/tests/test_thumbnail.py @@ -0,0 +1,320 @@ +# -*- coding: utf-8 -*- +from django.conf import settings + +from zerver.lib.test_classes import ZulipTestCase, UploadSerializeMixin +from zerver.lib.test_helpers import use_s3_backend, override_settings + +from io import StringIO +from boto.s3.connection import S3Connection +import ujson +import urllib +import base64 + +class ThumbnailTest(ZulipTestCase): + + @use_s3_backend + def test_s3_source_type(self) -> None: + def get_file_path_urlpart(uri: str, size: str='') -> str: + base = '/user_uploads/' + url_in_result = 'smart/filters:no_upscale()/%s/source_type/s3' + if size: + url_in_result = '/%s/%s' % (size, url_in_result) + upload_file_path = uri[len(base):] + hex_uri = base64.urlsafe_b64encode(upload_file_path.encode()).decode('utf-8') + return url_in_result % (hex_uri) + + conn = S3Connection(settings.S3_KEY, settings.S3_SECRET_KEY) + conn.create_bucket(settings.S3_AUTH_UPLOADS_BUCKET) + + self.login(self.example_email("hamlet")) + fp = StringIO("zulip!") + fp.name = "zulip.jpeg" + + result = self.client_post("/json/user_uploads", {'file': fp}) + self.assert_json_success(result) + json = ujson.loads(result.content) + self.assertIn("uri", json) + uri = json["uri"] + base = '/user_uploads/' + self.assertEqual(base, uri[:len(base)]) + + quoted_uri = urllib.parse.quote(uri[1:], safe='') + + # Test original image size. + result = self.client_get("/thumbnail?url=%s&size=original" % (quoted_uri)) + self.assertEqual(result.status_code, 302, result) + expected_part_url = get_file_path_urlpart(uri) + self.assertIn(expected_part_url, result.url) + + # Test thumbnail size. + result = self.client_get("/thumbnail?url=%s&size=thumbnail" % (quoted_uri)) + self.assertEqual(result.status_code, 302, result) + expected_part_url = get_file_path_urlpart(uri, '0x100') + self.assertIn(expected_part_url, result.url) + + # Tests the /api/v1/thumbnail api endpoint with standard API auth + self.logout() + result = self.api_get( + self.example_email("hamlet"), + '/thumbnail?url=%s&size=original' % + (quoted_uri,)) + self.assertEqual(result.status_code, 302, result) + expected_part_url = get_file_path_urlpart(uri) + self.assertIn(expected_part_url, result.url) + + # Test with another user trying to access image using thumbor. + self.login(self.example_email("iago")) + result = self.client_get("/thumbnail?url=%s&size=original" % (quoted_uri)) + self.assertEqual(result.status_code, 403, result) + self.assert_in_response("You are not authorized to view this file.", result) + + def test_external_source_type(self) -> None: + def run_test_with_image_url(image_url: str) -> None: + # Test original image size. + self.login(self.example_email("hamlet")) + quoted_url = urllib.parse.quote(image_url, safe='') + encoded_url = base64.urlsafe_b64encode(image_url.encode()).decode('utf-8') + result = self.client_get("/thumbnail?url=%s&size=original" % (quoted_url)) + self.assertEqual(result.status_code, 302, result) + expected_part_url = '/smart/filters:no_upscale()/' + encoded_url + '/source_type/external' + self.assertIn(expected_part_url, result.url) + + # Test thumbnail size. + result = self.client_get("/thumbnail?url=%s&size=thumbnail" % (quoted_url)) + self.assertEqual(result.status_code, 302, result) + expected_part_url = '/0x100/smart/filters:no_upscale()/' + encoded_url + '/source_type/external' + self.assertIn(expected_part_url, result.url) + + # Test api endpoint with standard API authentication. + self.logout() + user_profile = self.example_user("hamlet") + result = self.api_get(user_profile.email, + "/thumbnail?url=%s&size=thumbnail" % (quoted_url,)) + self.assertEqual(result.status_code, 302, result) + expected_part_url = '/0x100/smart/filters:no_upscale()/' + encoded_url + '/source_type/external' + self.assertIn(expected_part_url, result.url) + + # Test api endpoint with legacy API authentication. + user_profile = self.example_user("hamlet") + result = self.client_get("/thumbnail?url=%s&size=thumbnail&api_key=%s" % ( + quoted_url, user_profile.api_key)) + self.assertEqual(result.status_code, 302, result) + expected_part_url = '/0x100/smart/filters:no_upscale()/' + encoded_url + '/source_type/external' + self.assertIn(expected_part_url, result.url) + + # Test a second logged-in user; they should also be able to access it + user_profile = self.example_user("iago") + result = self.client_get("/thumbnail?url=%s&size=thumbnail&api_key=%s" % (quoted_url, user_profile.api_key)) + self.assertEqual(result.status_code, 302, result) + expected_part_url = '/0x100/smart/filters:no_upscale()/' + encoded_url + '/source_type/external' + self.assertIn(expected_part_url, result.url) + + # Test with another user trying to access image using thumbor. + # File should be always accessible to user in case of external source + self.login(self.example_email("iago")) + result = self.client_get("/thumbnail?url=%s&size=original" % (quoted_url)) + self.assertEqual(result.status_code, 302, result) + expected_part_url = '/smart/filters:no_upscale()/' + encoded_url + '/source_type/external' + self.assertIn(expected_part_url, result.url) + + image_url = 'https://images.foobar.com/12345' + run_test_with_image_url(image_url) + + image_url = 'http://images.foobar.com/12345' + run_test_with_image_url(image_url) + + def test_local_file_type(self) -> None: + def get_file_path_urlpart(uri: str, size: str='') -> str: + base = '/user_uploads/' + url_in_result = 'smart/filters:no_upscale()/%s/source_type/local_file' + if size: + url_in_result = '/%s/%s' % (size, url_in_result) + upload_file_path = uri[len(base):] + hex_uri = base64.urlsafe_b64encode(upload_file_path.encode()).decode('utf-8') + return url_in_result % (hex_uri) + + self.login(self.example_email("hamlet")) + fp = StringIO("zulip!") + fp.name = "zulip.jpeg" + + result = self.client_post("/json/user_uploads", {'file': fp}) + self.assert_json_success(result) + json = ujson.loads(result.content) + self.assertIn("uri", json) + uri = json["uri"] + base = '/user_uploads/' + self.assertEqual(base, uri[:len(base)]) + + # Test original image size. + # We remove the forward slash infront of the `/user_uploads/` to match + # bugdown behaviour. + quoted_uri = urllib.parse.quote(uri[1:], safe='') + result = self.client_get("/thumbnail?url=%s&size=original" % (quoted_uri)) + self.assertEqual(result.status_code, 302, result) + expected_part_url = get_file_path_urlpart(uri) + self.assertIn(expected_part_url, result.url) + + # Test thumbnail size. + result = self.client_get("/thumbnail?url=%s&size=thumbnail" % (quoted_uri)) + self.assertEqual(result.status_code, 302, result) + expected_part_url = get_file_path_urlpart(uri, '0x100') + self.assertIn(expected_part_url, result.url) + + # Test with a unicode filename. + fp = StringIO("zulip!") + fp.name = "μένει.jpg" + + result = self.client_post("/json/user_uploads", {'file': fp}) + self.assert_json_success(result) + json = ujson.loads(result.content) + self.assertIn("uri", json) + uri = json["uri"] + + # We remove the forward slash infront of the `/user_uploads/` to match + # bugdown behaviour. + quoted_uri = urllib.parse.quote(uri[1:], safe='') + result = self.client_get("/thumbnail?url=%s&size=original" % (quoted_uri)) + self.assertEqual(result.status_code, 302, result) + expected_part_url = get_file_path_urlpart(uri) + self.assertIn(expected_part_url, result.url) + self.logout() + + # Tests the /api/v1/thumbnail api endpoint with HTTP basic auth. + user_profile = self.example_user("hamlet") + result = self.api_get( + self.example_email("hamlet"), + '/thumbnail?url=%s&size=original' % + (quoted_uri,)) + self.assertEqual(result.status_code, 302, result) + expected_part_url = get_file_path_urlpart(uri) + self.assertIn(expected_part_url, result.url) + + # Tests the /api/v1/thumbnail api endpoint with ?api_key + # auth. + user_profile = self.example_user("hamlet") + result = self.client_get( + '/thumbnail?url=%s&size=original&api_key=%s' % + (quoted_uri, user_profile.api_key)) + self.assertEqual(result.status_code, 302, result) + expected_part_url = get_file_path_urlpart(uri) + self.assertIn(expected_part_url, result.url) + + # Test with another user trying to access image using thumbor. + self.login(self.example_email("iago")) + result = self.client_get("/thumbnail?url=%s&size=original" % (quoted_uri)) + self.assertEqual(result.status_code, 403, result) + self.assert_in_response("You are not authorized to view this file.", result) + + @override_settings(THUMBOR_URL='127.0.0.1:9995') + def test_with_static_files(self) -> None: + self.login(self.example_email("hamlet")) + uri = '/static/images/cute/turtle.png' + quoted_uri = urllib.parse.quote(uri[1:], safe='') + result = self.client_get("/thumbnail?url=%s&size=original" % (quoted_uri)) + self.assertEqual(result.status_code, 302, result) + self.assertEqual(uri, result.url) + + def test_with_thumbor_disabled(self) -> None: + self.login(self.example_email("hamlet")) + fp = StringIO("zulip!") + fp.name = "zulip.jpeg" + + result = self.client_post("/json/user_uploads", {'file': fp}) + self.assert_json_success(result) + json = ujson.loads(result.content) + self.assertIn("uri", json) + uri = json["uri"] + base = '/user_uploads/' + self.assertEqual(base, uri[:len(base)]) + + quoted_uri = urllib.parse.quote(uri[1:], safe='') + + with self.settings(THUMBOR_URL=''): + result = self.client_get("/thumbnail?url=%s&size=original" % (quoted_uri)) + self.assertEqual(result.status_code, 302, result) + self.assertEqual(uri, result.url) + + uri = 'https://www.google.com/images/srpr/logo4w.png' + quoted_uri = urllib.parse.quote(uri, safe='') + with self.settings(THUMBOR_URL=''): + result = self.client_get("/thumbnail?url=%s&size=original" % (quoted_uri)) + self.assertEqual(result.status_code, 302, result) + self.assertEqual(uri, result.url) + + uri = 'http://www.google.com/images/srpr/logo4w.png' + quoted_uri = urllib.parse.quote(uri, safe='') + with self.settings(THUMBOR_URL=''): + result = self.client_get("/thumbnail?url=%s&size=original" % (quoted_uri)) + self.assertEqual(result.status_code, 302, result) + base = 'https://external-content.zulipcdn.net/7b6552b60c635e41e8f6daeb36d88afc4eabde79/687474703a2f2f7777772e676f6f676c652e636f6d2f696d616765732f737270722f6c6f676f34772e706e67' + self.assertEqual(base, result.url) + + def test_with_different_THUMBOR_URL(self) -> None: + self.login(self.example_email("hamlet")) + fp = StringIO("zulip!") + fp.name = "zulip.jpeg" + + result = self.client_post("/json/user_uploads", {'file': fp}) + self.assert_json_success(result) + json = ujson.loads(result.content) + self.assertIn("uri", json) + uri = json["uri"] + base = '/user_uploads/' + self.assertEqual(base, uri[:len(base)]) + + quoted_uri = urllib.parse.quote(uri[1:], safe='') + hex_uri = base64.urlsafe_b64encode(uri[len('/user_uploads/'):].encode()).decode('utf-8') + with self.settings(THUMBOR_URL='http://test-thumborhost.com'): + result = self.client_get("/thumbnail?url=%s&size=original" % (quoted_uri)) + self.assertEqual(result.status_code, 302, result) + base = 'http://test-thumborhost.com/' + self.assertEqual(base, result.url[:len(base)]) + expected_part_url = '/smart/filters:no_upscale()/' + hex_uri + '/source_type/local_file' + self.assertIn(expected_part_url, result.url) + + def test_with_different_sizes(self) -> None: + def get_file_path_urlpart(uri: str, size: str='') -> str: + base = '/user_uploads/' + url_in_result = 'smart/filters:no_upscale()/%s/source_type/local_file' + if size: + url_in_result = '/%s/%s' % (size, url_in_result) + upload_file_path = uri[len(base):] + hex_uri = base64.urlsafe_b64encode(upload_file_path.encode()).decode('utf-8') + return url_in_result % (hex_uri) + + self.login(self.example_email("hamlet")) + fp = StringIO("zulip!") + fp.name = "zulip.jpeg" + + result = self.client_post("/json/user_uploads", {'file': fp}) + self.assert_json_success(result) + json = ujson.loads(result.content) + self.assertIn("uri", json) + uri = json["uri"] + base = '/user_uploads/' + self.assertEqual(base, uri[:len(base)]) + + # Test with size supplied as a query parameter. + # size=thumbnail should return a 0x100 sized image. + # size=original should return the original resolution image. + quoted_uri = urllib.parse.quote(uri[1:], safe='') + result = self.client_get("/thumbnail?url=%s&size=thumbnail" % (quoted_uri)) + self.assertEqual(result.status_code, 302, result) + expected_part_url = get_file_path_urlpart(uri, '0x100') + self.assertIn(expected_part_url, result.url) + + result = self.client_get("/thumbnail?url=%s&size=original" % (quoted_uri)) + self.assertEqual(result.status_code, 302, result) + expected_part_url = get_file_path_urlpart(uri) + self.assertIn(expected_part_url, result.url) + + # Test with size supplied as a query parameter where size is anything + # else than original or thumbnail. Result should be an error message. + result = self.client_get("/thumbnail?url=%s&size=480x360" % (quoted_uri)) + self.assertEqual(result.status_code, 403, result) + self.assert_in_response("Invalid size.", result) + + # Test with no size param supplied. In this case as well we show an + # error message. + result = self.client_get("/thumbnail?url=%s" % (quoted_uri)) + self.assertEqual(result.status_code, 400, "Missing 'size' argument") diff --git a/zerver/views/thumbnail.py b/zerver/views/thumbnail.py new file mode 100644 index 0000000000..c6ad1520fc --- /dev/null +++ b/zerver/views/thumbnail.py @@ -0,0 +1,38 @@ +# -*- coding: utf-8 -*- +from django.shortcuts import redirect +from django.utils.translation import ugettext as _ +from django.http import HttpRequest, HttpResponse, HttpResponseForbidden +from django.conf import settings +from typing import Optional +from zerver.models import UserProfile, validate_attachment_request +from zerver.lib.request import has_request_variables, REQ +from zerver.lib.thumbnail import generate_thumbnail_url +import urllib + +def validate_thumbnail_request(user_profile: UserProfile, path: str) -> Optional[bool]: + # path here does not have a leading / as it is parsed from request hitting the + # thumbnail endpoint (defined in urls.py) that way. + if path.startswith('user_uploads/'): + path_id = path[len('user_uploads/'):] + return validate_attachment_request(user_profile, path_id) + + # This is an external link and we don't enforce restricted view policy here. + return True + +@has_request_variables +def backend_serve_thumbnail(request: HttpRequest, user_profile: UserProfile, + url: str=REQ(), size_requested: str=REQ("size")) -> HttpResponse: + if not validate_thumbnail_request(user_profile, url): + return HttpResponseForbidden(_("

You are not authorized to view this file.

")) + + size = None + if size_requested == 'thumbnail': + size = '0x100' + elif size_requested == 'original': + size = '0x0' + + if size is None: + return HttpResponseForbidden(_("

Invalid size.

")) + + thumbnail_url = generate_thumbnail_url(url, size) + return redirect(thumbnail_url) diff --git a/zproject/dev_settings.py b/zproject/dev_settings.py index 26f2859774..f633acc3ec 100644 --- a/zproject/dev_settings.py +++ b/zproject/dev_settings.py @@ -87,3 +87,5 @@ SENDFILE_BACKEND = 'sendfile.backends.development' # Set this True to send all hotspots in development ALWAYS_SEND_ALL_HOTSPOTS = False # type: bool + +THUMBOR_URL = 'http://127.0.0.1:9995' diff --git a/zproject/prod_settings_template.py b/zproject/prod_settings_template.py index 7095096baa..9ca112ce5f 100644 --- a/zproject/prod_settings_template.py +++ b/zproject/prod_settings_template.py @@ -485,9 +485,9 @@ CAMO_URI = '/external_content/' # By default, Zulip connects to the thumbor (the thumbnailing software # we use) service running locally on the machine. If you're running # thumbor on a different server, you can configure that by setting -# THUMBOR_HOST here. Setting THUMBOR_HOST='' will disable +# THUMBOR_URL here. Setting THUMBOR_URL='' will disable # thumbnailing in Zulip. -#THUMBOR_HOST = '127.0.0.1:9995' +#THUMBOR_URL = 'http://127.0.0.1:9995' # Controls the Jitsi video call integration. By default, the # integration uses the SaaS meet.jit.si server. You can specify diff --git a/zproject/settings.py b/zproject/settings.py index 1e12064eb4..9b2cb8b928 100644 --- a/zproject/settings.py +++ b/zproject/settings.py @@ -191,7 +191,7 @@ DEFAULT_SETTINGS = { 'REDIS_PORT': 6379, 'REMOTE_POSTGRES_HOST': '', 'REMOTE_POSTGRES_SSLMODE': '', - 'THUMBOR_HOST': '', + 'THUMBOR_URL': '', 'SENDFILE_BACKEND': None, # ToS/Privacy templates diff --git a/zproject/test_settings.py b/zproject/test_settings.py index 1e3b7a0820..1bbc30d9c7 100644 --- a/zproject/test_settings.py +++ b/zproject/test_settings.py @@ -154,3 +154,5 @@ PUSH_NOTIFICATION_BOUNCER_URL = None # Disable messages from slow queries as they affect backend tests. SLOW_QUERY_MESSAGES_ENABLED = False + +THUMBOR_URL = 'http://127.0.0.1:9995' diff --git a/zproject/urls.py b/zproject/urls.py index 88d6f338e9..86cd610c4e 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -538,11 +538,17 @@ urls += url(r'^user_uploads/(?P(\d*|unk))/(?P.*)', rest_dispatch, {'GET': ('zerver.views.upload.serve_file_backend', {'override_api_url_scheme'})}), +# This endpoint serves thumbnailed versions of images using thumbor; +# it requires an exception for the same reason. +urls += url(r'^thumbnail', rest_dispatch, + {'GET': ('zerver.views.thumbnail.backend_serve_thumbnail', + {'override_api_url_scheme'})}), # This url serves as a way to recieve CSP violation reports from the users. # We use this endpoint to just log these reports. urls += url(r'^report/csp_violations$', zerver.views.report.report_csp_violations, name='zerver.views.report.report_csp_violations'), + # Incoming webhook URLs # We don't create urls for particular git integrations here # because of generic one below diff --git a/zthumbor/loaders/helpers.py b/zthumbor/loaders/helpers.py index e7920bb362..e25d1d9c4b 100644 --- a/zthumbor/loaders/helpers.py +++ b/zthumbor/loaders/helpers.py @@ -1,13 +1,10 @@ from __future__ import absolute_import import os +import re import sys -import hmac -import time -import base64 -from hashlib import sha1 -from six.moves.urllib.parse import urlparse, parse_qs -from typing import Any, AnyStr, Dict, List, Optional, Text, Union +from six.moves.urllib.parse import urlparse +from typing import Any, Text, Tuple, Optional if False: from thumbor.context import Context @@ -44,43 +41,8 @@ THUMBOR_EXTERNAL_TYPE = 'external' THUMBOR_S3_TYPE = 's3' THUMBOR_LOCAL_FILE_TYPE = 'local_file' -def force_text(s, encoding='utf-8'): - # type: (Union[Text, bytes], str) -> Text - """converts a string to a text string""" - if isinstance(s, Text): - return s - elif isinstance(s, bytes): - return s.decode(encoding) - else: - raise TypeError("force_text expects a string type") - -def get_sign_hash(raw, key): - # type: (Text, Text) -> Text - hashed = hmac.new(key.encode('utf-8'), raw.encode('utf-8'), sha1) - return base64.b64encode(hashed.digest()).decode() - -def get_url_params(url): - # type: (Text) -> Dict[str, Any] - data = parse_qs(urlparse(url).query) - return {k: v[0] for k, v in data.items() if v} - -def sign_is_valid(url, context): - # type: (str, Context) -> bool - size = '{0}x{1}'.format(context.request.width, context.request.height) - data = parse_qs(urlparse(url).query) - source_type = data.get('source_type', [''])[0] - sign = data.get('sign', [''])[0] - if not source_type or not sign: - return False - url_path = url.rsplit('?', 1)[0] - if url_path.startswith('files/'): - url_path = url_path.split('/', 1)[1] - raw = u'_'.join([ - force_text(url_path), - force_text(size), - force_text(source_type), - ]) - secret_key = get_secret('thumbor_key') - if secret_key is None or sign != get_sign_hash(raw, secret_key): - return False - return True +def separate_url_and_source_type(url): + # type: (Text) -> Tuple[Text, Text] + THUMBNAIL_URL_PATT = re.compile('^(?P.+)/source_type/(?P.+)') + matches = THUMBNAIL_URL_PATT.match(url) + return (matches.group('source_type'), matches.group('actual_url')) diff --git a/zthumbor/loaders/zloader.py b/zthumbor/loaders/zloader.py index ccaf8d3046..afdfa43372 100644 --- a/zthumbor/loaders/zloader.py +++ b/zthumbor/loaders/zloader.py @@ -2,16 +2,19 @@ from __future__ import absolute_import from six.moves import urllib from tornado.concurrent import return_future -from thumbor.loaders import LoaderResult, file_loader, http_loader +from thumbor.loaders import LoaderResult, file_loader, https_loader from tc_aws.loaders import s3_loader from thumbor.context import Context from .helpers import ( - get_url_params, sign_is_valid, THUMBOR_S3_TYPE, THUMBOR_LOCAL_FILE_TYPE, - THUMBOR_EXTERNAL_TYPE + separate_url_and_source_type, + THUMBOR_S3_TYPE, THUMBOR_LOCAL_FILE_TYPE, THUMBOR_EXTERNAL_TYPE ) from typing import Any, Callable +import base64 +import logging + def get_not_found_result(): # type: () -> LoaderResult result = LoaderResult() @@ -22,23 +25,18 @@ def get_not_found_result(): @return_future def load(context, url, callback): # type: (Context, str, Callable[..., Any]) -> None - url = urllib.parse.unquote(url) - url_params = get_url_params(url) - source_type = url_params.get('source_type') - - if not sign_is_valid(url, context) or source_type not in ( - THUMBOR_S3_TYPE, THUMBOR_LOCAL_FILE_TYPE, THUMBOR_EXTERNAL_TYPE): + source_type, encoded_url = separate_url_and_source_type(url) + actual_url = base64.urlsafe_b64decode(urllib.parse.unquote(encoded_url)) + if source_type not in (THUMBOR_S3_TYPE, THUMBOR_LOCAL_FILE_TYPE, + THUMBOR_EXTERNAL_TYPE): callback(get_not_found_result()) + logging.warning('INVALID SOURCE TYPE: ' + source_type) return - url = url.rsplit('?', 1)[0] if source_type == THUMBOR_S3_TYPE: - s3_loader.load(context, url, callback) + s3_loader.load(context, actual_url, callback) elif source_type == THUMBOR_LOCAL_FILE_TYPE: - file_loader.load(context, url, callback) + patched_local_url = 'files/' + actual_url # type: ignore # python 2 type differs from python 3 type + file_loader.load(context, patched_local_url, callback) elif source_type == THUMBOR_EXTERNAL_TYPE: - http_loader.load_sync( - context, - url, - callback, - normalize_url_func=http_loader._normalize_url) + https_loader.load(context, actual_url, callback)