From 71124eb2addbebba986a576bf817b78052b9c06c Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Apr 17 2018 13:01:38 +0000 Subject: [PATCH 1/3] Do not syntax highlight 'huge' files Signed-off-by: Patrick Uiterwijk --- diff --git a/pagure/templates/file.html b/pagure/templates/file.html index 00329d4..3cfa6dc 100644 --- a/pagure/templates/file.html +++ b/pagure/templates/file.html @@ -176,7 +176,11 @@ {% if output_type=='file' %} {% autoescape false %} - {{ content | format_loc }} + {% if huge %} + {{ content | e | format_loc }} + {% else %} + {{ content | format_loc }} + {% endif %} {% endautoescape %} {% elif output_type == 'markup' %}
diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 6e14d0c..dfdc5db 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -56,6 +56,7 @@ from pagure.utils import ( __get_file_in_tree, authenticated, login_required, + stream_template, ) from pagure.decorators import ( is_repo_admin, @@ -65,6 +66,11 @@ from pagure.decorators import ( _log = logging.getLogger(__name__) +# Number of characters to determine that a file is "huge" +# Huge files will not get syntax highlighting +HUGEFILE = 5000 + + def get_git_url_ssh(): """ Return the GIT SSH URL to be displayed in the UI based on the content of the configuration file. @@ -526,6 +532,11 @@ def view_file(repo, identifier, filename, username=None, namespace=None): safe = False readme_ext = None headers = {} + huge = False + + isbinary = False + if 'data' in dir(content): + isbinary = is_binary_string(content.data) if isinstance(content, pygit2.Blob): rawtext = str(flask.request.args.get('text')).lower() in ['1', 'true'] @@ -544,7 +555,7 @@ def view_file(repo, identifier, filename, username=None, namespace=None): elif ext in ('.rst', '.mk', '.md', '.markdown') and not rawtext: content, safe = pagure.doc_utils.convert_readme(content.data, ext) output_type = 'markup' - elif not is_binary_string(content.data): + elif 'data' in dir(content) and len(content.data) < HUGEFILE and not isbinary: file_content = None try: file_content = encoding_utils.decode( @@ -579,6 +590,11 @@ def view_file(repo, identifier, filename, username=None, namespace=None): output_type = 'file' else: output_type = 'binary' + elif not isbinary: + output_type = 'file' + huge = True + safe = False + content = content.data.decode('utf-8') else: output_type = 'binary' elif isinstance(content, pygit2.Commit): @@ -600,8 +616,8 @@ def view_file(repo, identifier, filename, username=None, namespace=None): if output_type == 'binary': headers['Content-Disposition'] = 'attachment' - return ( - flask.render_template( + return flask.Response(flask.stream_with_context(stream_template( + flask.current_app, 'file.html', select='tree', repo=repo, @@ -614,7 +630,8 @@ def view_file(repo, identifier, filename, username=None, namespace=None): readme=readme, readme_ext=readme_ext, safe=safe, - ), + huge=huge, + )), 200, headers ) diff --git a/pagure/utils.py b/pagure/utils.py index 057ddeb..0c9222a 100644 --- a/pagure/utils.py +++ b/pagure/utils.py @@ -389,3 +389,11 @@ def get_parent_repo_path(repo): parentpath = os.path.join(pagure_config['GIT_FOLDER'], repo.path) return parentpath + + +def stream_template(app, template_name, **context): + app.update_template_context(context) + t = app.jinja_env.get_template(template_name) + rv = t.stream(context) + rv.enable_buffering(5) + return rv From 5ff6c025443d6f14089b8beb76b43f857b2cef23 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Apr 17 2018 13:01:38 +0000 Subject: [PATCH 2/3] Move end_request to teardown_request This means that database session removal happens after the full response is sent back to the browser, meaning we can use streaming. Signed-off-by: Patrick Uiterwijk --- diff --git a/pagure/flask_app.py b/pagure/flask_app.py index f898d9b..acbf2b8 100644 --- a/pagure/flask_app.py +++ b/pagure/flask_app.py @@ -145,7 +145,7 @@ def create_app(config=None): app.register_blueprint(PV) app.before_request(set_request) - app.after_request(end_request) + app.teardown_request(end_request) # Only import the login controller if the app is set up for local login if pagure_config.get('PAGURE_AUTH', None) == 'local': @@ -389,7 +389,8 @@ def auth_logout(): # pragma: no cover return flask.redirect(return_point) -def end_request(response): +# pylint: disable=unused-argument +def end_request(exception=None): """ This method is called at the end of each request. Remove the DB session at the end of each request. @@ -400,8 +401,6 @@ def end_request(response): flask.g.session.remove() gc.collect() - return response - def _get_user(username): """ Check if user exists or not From efbef738e475af08c5f1a810e7b66788b1917808 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Apr 17 2018 13:01:38 +0000 Subject: [PATCH 3/3] Fix unit-tests Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index dfdc5db..385a9ab 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -555,7 +555,9 @@ def view_file(repo, identifier, filename, username=None, namespace=None): elif ext in ('.rst', '.mk', '.md', '.markdown') and not rawtext: content, safe = pagure.doc_utils.convert_readme(content.data, ext) output_type = 'markup' - elif 'data' in dir(content) and len(content.data) < HUGEFILE and not isbinary: + elif 'data' in dir(content) \ + and len(content.data) < HUGEFILE \ + and not isbinary: file_content = None try: file_content = encoding_utils.decode( @@ -616,7 +618,8 @@ def view_file(repo, identifier, filename, username=None, namespace=None): if output_type == 'binary': headers['Content-Disposition'] = 'attachment' - return flask.Response(flask.stream_with_context(stream_template( + return flask.Response(flask.stream_with_context( + stream_template( flask.current_app, 'file.html', select='tree', diff --git a/tests/test_pagure_flask_form.py b/tests/test_pagure_flask_form.py index 5778bd3..a45a7a9 100644 --- a/tests/test_pagure_flask_form.py +++ b/tests/test_pagure_flask_form.py @@ -19,7 +19,7 @@ import os import flask import flask_wtf -from mock import patch +from mock import patch, MagicMock sys.path.insert(0, os.path.join(os.path.dirname( os.path.abspath(__file__)), '..')) @@ -38,12 +38,14 @@ class PagureFlaskFormTests(tests.SimplePagureTest): def test_csrf_form_no_input(self): """ Test the CSRF validation if not CSRF is specified. """ with self.app.application.test_request_context(method='POST'): + flask.g.session = MagicMock() form = pagure.forms.ConfirmationForm() self.assertFalse(form.validate_on_submit()) def test_csrf_form_w_invalid_input(self): """ Test the CSRF validation with an invalid CSRF specified. """ with self.app.application.test_request_context(method='POST'): + flask.g.session = MagicMock() form = pagure.forms.ConfirmationForm() form.csrf_token.data = 'foobar' self.assertFalse(form.validate_on_submit()) @@ -51,6 +53,7 @@ class PagureFlaskFormTests(tests.SimplePagureTest): def test_csrf_form_w_input(self): """ Test the CSRF validation with a valid CSRF specified. """ with self.app.application.test_request_context(method='POST'): + flask.g.session = MagicMock() form = pagure.forms.ConfirmationForm() form.csrf_token.data = form.csrf_token.current_token self.assertTrue(form.validate_on_submit()) @@ -58,6 +61,7 @@ class PagureFlaskFormTests(tests.SimplePagureTest): def test_csrf_form_w_expired_input(self): """ Test the CSRF validation with an expired CSRF specified. """ with self.app.application.test_request_context(method='POST'): + flask.g.session = MagicMock() form = pagure.forms.ConfirmationForm() data = form.csrf_token.current_token @@ -90,6 +94,7 @@ class PagureFlaskFormTests(tests.SimplePagureTest): """ Test the CSRF validation with a CSRF not expiring. """ pagure.config.config['WTF_CSRF_TIME_LIMIT'] = None with self.app.application.test_request_context(method='POST'): + flask.g.session = MagicMock() form = pagure.forms.ConfirmationForm() data = form.csrf_token.current_token @@ -106,6 +111,7 @@ class PagureFlaskFormTests(tests.SimplePagureTest): def test_add_user_form(self): """ Test the AddUserForm of pagure.forms """ with self.app.application.test_request_context(method='POST'): + flask.g.session = MagicMock() form = pagure.forms.AddUserForm() form.csrf_token.data = form.csrf_token.current_token # No user or access given @@ -119,6 +125,7 @@ class PagureFlaskFormTests(tests.SimplePagureTest): def test_add_user_to_group_form(self): """ Test the AddUserToGroup form of pagure.forms """ with self.app.application.test_request_context(method='POST'): + flask.g.session = MagicMock() form = pagure.forms.AddUserToGroupForm() form.csrf_token.data = form.csrf_token.current_token # No user given @@ -130,6 +137,7 @@ class PagureFlaskFormTests(tests.SimplePagureTest): def test_add_group_form(self): """ Test the AddGroupForm form of pagure.forms """ with self.app.application.test_request_context(method='POST'): + flask.g.session = MagicMock() form = pagure.forms.AddGroupForm() form.csrf_token.data = form.csrf_token.current_token # No group given