[#3244|Web] Add support for accept-encoding header

* Use EncodingResourceWrapper to replace compress function so that the proper checks for accept-encoding header are made. * Ensure only text is compressed and images are left uncompressed.
DanielHuey · May 9, 2019 · db021b9 · db021b9
1 parent ab4661f
commit db021b9
Show file tree

Hide file tree

Showing 9 changed files with 65 additions and 86 deletions.
diff --git a/deluge/tests/common_web.py b/deluge/tests/common_web.py
@@ -67,10 +67,3 @@ def check_request(request, method=None, level=None):
             pass
 
         self.patch(auth, 'check_request', check_request)
-
-    def mock_compress_body(self):
-        def compress(contents, request):
-            return contents
-
-        # Patch compress to avoid having to decompress output with zlib
-        self.patch(deluge.ui.web.json_api, 'compress', compress)
diff --git a/deluge/tests/test_core.py b/deluge/tests/test_core.py
@@ -16,8 +16,8 @@
 from twisted.internet.error import CannotListenError
 from twisted.python.failure import Failure
 from twisted.web.http import FORBIDDEN
-from twisted.web.resource import Resource
-from twisted.web.server import Site
+from twisted.web.resource import EncodingResourceWrapper, Resource
+from twisted.web.server import GzipEncoderFactory, Site
 from twisted.web.static import File
 
 import deluge.common
@@ -26,7 +26,6 @@
 from deluge.core.core import Core
 from deluge.core.rpcserver import RPCServer
 from deluge.error import AddTorrentError, InvalidTorrentError
-from deluge.ui.web.common import compress
 
 from . import common
 from .basetest import BaseTestCase
@@ -49,15 +48,16 @@ def render(self, request):
 
 
 class PartialDownload(Resource):
+    def getChild(self, path, request):  # NOQA: N802
+        return EncodingResourceWrapper(self, [GzipEncoderFactory()])
+
     def render(self, request):
         with open(
             common.get_test_data_file('ubuntu-9.04-desktop-i386.iso.torrent'), 'rb'
         ) as _file:
             data = _file.read()
         request.setHeader(b'Content-Length', str(len(data)))
         request.setHeader(b'Content-Type', b'application/x-bittorrent')
-        if request.requestHeaders.hasHeader('accept-encoding'):
-            return compress(data, request)
         return data
 
 

diff --git a/deluge/tests/test_httpdownloader.py b/deluge/tests/test_httpdownloader.py
@@ -16,14 +16,13 @@
 from twisted.trial import unittest
 from twisted.web.error import PageRedirect
 from twisted.web.http import NOT_MODIFIED
-from twisted.web.resource import Resource
-from twisted.web.server import Site
+from twisted.web.resource import EncodingResourceWrapper, Resource
+from twisted.web.server import GzipEncoderFactory, Site
 from twisted.web.util import redirectTo
 
 from deluge.common import windows_check
 from deluge.httpdownloader import download_file
 from deluge.log import setup_logger
-from deluge.ui.web.common import compress
 
 temp_dir = tempfile.mkdtemp()
 
@@ -66,10 +65,13 @@ def render(self, request):
 
 
 class GzipResource(Resource):
+    def getChild(self, path, request):  # NOQA: N802
+        return EncodingResourceWrapper(self, [GzipEncoderFactory()])
+
     def render(self, request):
         message = request.args.get(b'msg', [b'EFFICIENCY!'])[0]
         request.setHeader(b'Content-Type', b'text/plain')
-        return compress(message, request)
+        return message
 
 
 class PartialDownloadResource(Resource):
@@ -227,13 +229,9 @@ def test_download_with_gzip_encoding(self):
         return d
 
     def test_download_with_gzip_encoding_disabled(self):
-        url = self.get_url('gzip?msg=fail')
+        url = self.get_url('gzip?msg=unzip')
         d = download_file(url, fname('gzip_encoded'), allow_compression=False)
-
-        def cb(result):
-            print(result)
-
-        d.addCallback(self.assertNotContains, b'fail', file_mode='rb')
+        d.addCallback(self.assertContains, 'unzip')
         return d
 
     def test_page_redirect_unhandled(self):

diff --git a/deluge/tests/test_json_api.py b/deluge/tests/test_json_api.py
@@ -79,11 +79,6 @@ def test_render_fail(self):
         request = MagicMock()
         request.method = b'POST'
 
-        def compress(contents, request):
-            return contents
-
-        self.patch(deluge.ui.web.json_api, 'compress', compress)
-
         def write(response_str):
             request.write_was_called = True
             response = json_lib.loads(response_str.decode())
@@ -267,7 +262,6 @@ def test_render_on_rpc_request_failed(self):
         # Circumvent authentication
         auth = Auth({})
         self.mock_authentication_ignore(auth)
-        self.mock_compress_body()
 
         def write(response_str):
             request.write_was_called = True

diff --git a/deluge/tests/test_webserver.py b/deluge/tests/test_webserver.py
@@ -31,7 +31,6 @@ def test_get_torrent_info(self):
         agent = Agent(reactor)
 
         self.mock_authentication_ignore(self.deluge_web.auth)
-        self.mock_compress_body()
 
         # This torrent file contains an uncommon field 'filehash' which must be hex
         # encoded to allow dumping the torrent info to json. Otherwise it will fail with:

diff --git a/deluge/ui/web/common.py b/deluge/ui/web/common.py
@@ -10,7 +10,8 @@
 from __future__ import unicode_literals
 
 import gettext
-import zlib
+
+from mako.template import Template as MakoTemplate
 
 from deluge.common import PY2, get_version
 
@@ -34,39 +35,14 @@ def escape(text):
     return text
 
 
-def compress(contents, request):
-    request.setHeader(b'content-encoding', b'gzip')
-    compress_zlib = zlib.compressobj(
-        6, zlib.DEFLATED, zlib.MAX_WBITS + 16, zlib.DEF_MEM_LEVEL, 0
-    )
-    contents = compress_zlib.compress(contents)
-    contents += compress_zlib.flush()
-    return contents
-
-
-try:
-    # This is beeing done like this in order to allow tests to use the above
-    # `compress` without requiring Mako to be instaled
-    from mako.template import Template as MakoTemplate
-
-    class Template(MakoTemplate):
-        """
-        A template that adds some built-ins to the rendering
-        """
-
-        builtins = {'_': _, 'escape': escape, 'version': get_version()}
-
-        def render(self, *args, **data):
-            data.update(self.builtins)
-            rendered = MakoTemplate.render_unicode(self, *args, **data)
-            return rendered.encode('utf-8')
-
-
-except ImportError:
-    import warnings
+class Template(MakoTemplate):
+    """
+    A template that adds some built-ins to the rendering
+    """
 
-    warnings.warn('The Mako library is required to run deluge.ui.web', RuntimeWarning)
+    builtins = {'_': _, 'escape': escape, 'version': get_version()}
 
-    class Template(object):
-        def __new__(cls, *args, **kwargs):
-            raise RuntimeError('The Mako library is required to run deluge.ui.web')
+    def render(self, *args, **data):
+        data.update(self.builtins)
+        rendered = MakoTemplate.render_unicode(self, *args, **data)
+        return rendered.encode('utf-8')
diff --git a/deluge/ui/web/json_api.py b/deluge/ui/web/json_api.py
@@ -32,7 +32,7 @@
 from deluge.ui.hostlist import HostList
 from deluge.ui.sessionproxy import SessionProxy
 from deluge.ui.translations_util import get_languages
-from deluge.ui.web.common import _, compress
+from deluge.ui.web.common import _
 
 log = logging.getLogger(__name__)
 
@@ -231,7 +231,7 @@ def _send_response(self, request, response):
             return ''
         response = json.dumps(response)
         request.setHeader(b'content-type', b'application/json')
-        request.write(compress(response.encode(), request))
+        request.write(response.encode())
         request.finish()
         return server.NOT_DONE_YET
 

diff --git a/deluge/ui/web/server.py b/deluge/ui/web/server.py
@@ -19,6 +19,7 @@
 from twisted.application import internet, service
 from twisted.internet import defer, reactor
 from twisted.web import http, resource, server, static
+from twisted.web.resource import EncodingResourceWrapper
 
 from deluge import common, component, configmanager
 from deluge.common import is_ipv6
@@ -27,7 +28,7 @@
 from deluge.ui.tracker_icons import TrackerIcons
 from deluge.ui.translations_util import set_language, setup_translations
 from deluge.ui.web.auth import Auth
-from deluge.ui.web.common import Template, compress
+from deluge.ui.web.common import Template
 from deluge.ui.web.json_api import JSON, WebApi, WebUtils
 from deluge.ui.web.pluginmanager import PluginManager
 
@@ -80,7 +81,7 @@ class GetText(resource.Resource):
     def render(self, request):
         request.setHeader(b'content-type', b'text/javascript; encoding=utf-8')
         template = Template(filename=rpath('js', 'gettext.js'))
-        return compress(template.render(), request)
+        return template.render()
 
 
 class MockGetText(resource.Resource):
@@ -93,8 +94,7 @@ class MockGetText(resource.Resource):
 
     def render(self, request):
         request.setHeader(b'content-type', b'text/javascript; encoding=utf-8')
-        data = b'function _(string) { return string; }'
-        return compress(data, request)
+        return b'function _(string) { return string; }'
 
 
 class Upload(resource.Resource):
@@ -131,9 +131,8 @@ def render(self, request):
 
         request.setHeader(b'content-type', b'text/html')
         request.setResponseCode(http.OK)
-        return compress(
-            json.dumps({'success': bool(filenames), 'files': filenames}).encode('utf8'),
-            request,
+        return json.dumps({'success': bool(filenames), 'files': filenames}).encode(
+            'utf8'
         )
 
 
@@ -145,7 +144,7 @@ def __init__(self):
 
     def getChild(self, path, request):  # NOQA: N802
         request.render_file = path
-        return self
+        return EncodingResourceWrapper(self, [server.GzipEncoderFactory()])
 
     def render(self, request):
         log.debug('Render template file: %s', request.render_file)
@@ -163,7 +162,7 @@ def render(self, request):
             tpl_file = '404.html'
 
         template = Template(filename=rpath(os.path.join('render', tpl_file)))
-        return compress(template.render(), request)
+        return template.render()
 
 
 class Tracker(resource.Resource):
@@ -242,7 +241,11 @@ def getChild(self, path, request):  # NOQA: N802
             request.lookup_path = os.path.join(request.lookup_path, path)
         else:
             request.lookup_path = path
-        return self
+
+        if request.uri.endswith(b'css'):
+            return EncodingResourceWrapper(self, [server.GzipEncoderFactory()])
+        else:
+            return self
 
     def render(self, request):
         log.debug('Requested path: %s', request.lookup_path)
@@ -258,12 +261,12 @@ def render(self, request):
                     request.setHeader(b'content-type', mime_type[0].encode())
                     with open(path, 'rb') as _file:
                         data = _file.read()
-                    return compress(data, request)
+                    return data
 
         request.setResponseCode(http.NOT_FOUND)
         request.setHeader(b'content-type', b'text/html')
         template = Template(filename=rpath(os.path.join('render', '404.html')))
-        return compress(template.render(), request)
+        return template.render()
 
 
 class ScriptResource(resource.Resource, component.Component):
@@ -404,7 +407,7 @@ def getChild(self, path, request):  # NOQA: N802
             request.lookup_path += b'/' + path
         else:
             request.lookup_path = path
-        return self
+        return EncodingResourceWrapper(self, [server.GzipEncoderFactory()])
 
     def render(self, request):
         log.debug('Requested path: %s', request.lookup_path)
@@ -429,12 +432,21 @@ def render(self, request):
                 request.setHeader(b'content-type', mime_type[0].encode())
                 with open(path, 'rb') as _file:
                     data = _file.read()
-                return compress(data, request)
+                return data
 
         request.setResponseCode(http.NOT_FOUND)
         request.setHeader(b'content-type', b'text/html')
         template = Template(filename=rpath(os.path.join('render', '404.html')))
-        return compress(template.render(), request)
+        return template.render()
+
+
+class Themes(static.File):
+    def getChild(self, path, request):  # NOQA: N802
+        child = static.File.getChild(self, path, request)
+        if request.uri.endswith(b'css'):
+            return EncodingResourceWrapper(child, [server.GzipEncoderFactory()])
+        else:
+            return child
 
 
 class TopLevel(resource.Resource):
@@ -450,7 +462,10 @@ def __init__(self):
 
         self.putChild(b'css', LookupResource('Css', rpath('css')))
         if os.path.isfile(rpath('js', 'gettext.js')):
-            self.putChild(b'gettext.js', GetText())
+            self.putChild(
+                b'gettext.js',
+                EncodingResourceWrapper(GetText(), [server.GzipEncoderFactory()]),
+            )
         else:
             log.warning(
                 'Cannot find "gettext.js" translation file!'
@@ -505,10 +520,14 @@ def __init__(self):
 
         self.js = js
         self.putChild(b'js', js)
-        self.putChild(b'json', JSON())
-        self.putChild(b'upload', Upload())
+        self.putChild(
+            b'json', EncodingResourceWrapper(JSON(), [server.GzipEncoderFactory()])
+        )
+        self.putChild(
+            b'upload', EncodingResourceWrapper(Upload(), [server.GzipEncoderFactory()])
+        )
         self.putChild(b'render', Render())
-        self.putChild(b'themes', static.File(rpath('themes')))
+        self.putChild(b'themes', Themes(rpath('themes')))
         self.putChild(b'tracker', Tracker())
 
         theme = component.get('DelugeWeb').config['theme']

diff --git a/setup.cfg b/setup.cfg
@@ -26,7 +26,7 @@ known_third_party =
     cairo, gi,
 # Ignore other module dependencies for pre-commit isort.
     twisted, OpenSSL, pytest, recommonmark, chardet, pkg_resources, zope, mock,
-    sphinx, rencode, six
+    sphinx, rencode, six, mako
 known_first_party = msgfmt, deluge
 order_by_type = true
 not_skip = __init__.py