This is an automated email from the ASF dual-hosted git repository. brondsem pushed a commit to branch db/8500 in repository https://gitbox.apache.org/repos/asf/allura.git
commit b6521e89883bea89aacfc4d13f9325a430378a88 Author: Dave Brondsema <[email protected]> AuthorDate: Thu Feb 16 15:43:48 2023 -0500 [#8500] upgrade Turbogears --- Allura/allura/command/base.py | 5 +- Allura/allura/config/app_cfg.py | 56 +++++++++++------- Allura/allura/config/environment.py | 25 -------- Allura/allura/config/middleware.py | 38 ++++-------- Allura/allura/lib/custom_middleware.py | 92 ++++++++++++++++++++++++++++- Allura/allura/lib/helpers.py | 2 +- Allura/allura/tests/decorators.py | 4 +- Allura/allura/tests/functional/test_root.py | 39 ++++++++++++ Allura/allura/websetup/__init__.py | 8 ++- Allura/development.ini | 1 - AlluraTest/alluratest/test_syntax.py | 2 +- requirements.in | 2 +- requirements.txt | 2 +- 13 files changed, 189 insertions(+), 87 deletions(-) diff --git a/Allura/allura/command/base.py b/Allura/allura/command/base.py index 0d66564f0..efa3add20 100644 --- a/Allura/allura/command/base.py +++ b/Allura/allura/command/base.py @@ -28,7 +28,7 @@ from tg.wsgiapp import RequestLocals import activitystream import ming -from allura.config.environment import load_environment +from allura.config.app_cfg import base_config from allura.lib.decorators import task from allura.lib import helpers as h from allura.lib.utils import configure_ming @@ -99,7 +99,8 @@ class Command(command.Command, metaclass=MetaParserDocstring): logging_config, disable_existing_loggers=False) log = logging.getLogger('allura.command') log.info('Initialize command with config %r', self.args[0]) - load_environment(conf.global_conf, conf.local_conf) + conf = base_config.configure(conf.global_conf, conf.local_conf) + base_config.setup(conf) self.setup_globals() from allura import model M = model diff --git a/Allura/allura/config/app_cfg.py b/Allura/allura/config/app_cfg.py index 732525c13..c00f5ab81 100644 --- a/Allura/allura/config/app_cfg.py +++ b/Allura/allura/config/app_cfg.py @@ -29,15 +29,17 @@ convert them into boolean, for example, you should use the """ import logging -from functools import partial -import six import sys import tg -from tg import app_globals as g from tg.renderers.jinja import JinjaRenderer +from tg.renderers.mako import MakoRenderer +from tg.renderers.json import JSONRenderer +from tg import MinimalApplicationConfigurator +from tg.configurator.components.rendering import TemplateRenderingConfigurationComponent +from tg.configuration import config + import jinja2 -from tg.configuration import AppConfig, config from markupsafe import Markup import ew from tg.support.converters import asint @@ -50,25 +52,37 @@ from allura.lib.package_path_loader import PackagePathLoader log = logging.getLogger(__name__) -class ForgeConfig(AppConfig): +class ForgeConfig(MinimalApplicationConfigurator): def __init__(self, root_controller=None): - AppConfig.__init__(self, minimal=True, root_controller=root_controller) - self.package = allura - self.renderers = ['json', 'mako', 'jinja'] - self.default_renderer = 'jinja' - self.register_rendering_engine(AlluraJinjaRenderer) - self.use_sqlalchemy = False - self.use_toscawidgets = False - self.use_transaction_manager = False - self['tm.enabled'] = False - self.handle_status_codes = [403, 404, 410] - self.disable_request_extensions = True - - # if left to True (default) would use crank.util.default_path_translator to convert all URL punctuation to "_" - # which is convenient for /foo-bar to execute a "def foo_bar" method, but is a pretty drastic change for us - # and makes many URLs be valid that we might not want like /foo*bar /foo@bar /foo:bar - self.dispatch_path_translator = None + super().__init__() + + self.update_blueprint({ + 'package': allura, + 'root_controller': root_controller, # optional override, otherwise default will be found in allura package + 'renderers': ['json', 'mako', 'jinja'], + 'default_renderer': 'jinja', + + # prevent dispatcher from striping extensions like '.io' from URLs + 'disable_request_extensions': True, + + # if left to True (default) would use crank.util.default_path_translator to convert all URL punctuation to "_" + # which is convenient for /foo-bar to execute a "def foo_bar" method, but is a pretty drastic change for us + # and makes many URLs be valid that we might not want like /foo*bar /foo@bar /foo:bar + 'dispatch_path_translator': None, + + }) + self.replace(TemplateRenderingConfigurationComponent.id, AlluraTemplateConfig) + + +class AlluraTemplateConfig(TemplateRenderingConfigurationComponent): + + def on_bind(self, configurator: ForgeConfig): + # no super, only register a few (other ones aren't a problem but are unnecessary, so this is optimal) + self.register_engine(JSONRenderer) + self.register_engine(MakoRenderer) + # this is *our* JinjaRenderer, not the default one + self.register_engine(AlluraJinjaRenderer) class AlluraJinjaRenderer(JinjaRenderer): diff --git a/Allura/allura/config/environment.py b/Allura/allura/config/environment.py deleted file mode 100644 index 4fdc823e9..000000000 --- a/Allura/allura/config/environment.py +++ /dev/null @@ -1,25 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -"""WSGI environment setup for allura.""" - -from allura.config.app_cfg import base_config - -__all__ = ['load_environment'] - -# Use base_config to setup the environment loader function -load_environment = base_config.make_load_environment() diff --git a/Allura/allura/config/middleware.py b/Allura/allura/config/middleware.py index 61fdaa09c..e10a14613 100644 --- a/Allura/allura/config/middleware.py +++ b/Allura/allura/config/middleware.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. -"""WSGI middleware initialization for the allura application.""" +"""core app and WSGI middleware initialization. New TurboGears2 apps name it application.py""" import ast import importlib @@ -23,12 +23,10 @@ import mimetypes import pickle import six import tg -import tg.error import pkg_resources from tg import config from paste.deploy.converters import asbool, aslist, asint from tg.support.registry import RegistryManager -from tg.support.middlewares import StatusCodeRedirect from beaker.middleware import SessionMiddleware from beaker.util import PickleSerializer from paste.exceptions.errormiddleware import ErrorMiddleware @@ -51,8 +49,7 @@ except ImportError: else: patches.newrelic() -from allura.config.app_cfg import base_config, AlluraJinjaRenderer # noqa: E402 -from allura.config.environment import load_environment # noqa: E402 +from allura.config.app_cfg import AlluraJinjaRenderer # noqa: E402 from allura.config.app_cfg import ForgeConfig # noqa: E402 from allura.lib.custom_middleware import AlluraTimerMiddleware # noqa: E402 from allura.lib.custom_middleware import SSLMiddleware # noqa: E402 @@ -64,13 +61,15 @@ from allura.lib.custom_middleware import RememberLoginMiddleware # noqa: E402 from allura.lib.custom_middleware import SetRequestHostFromConfig # noqa: E402 from allura.lib.custom_middleware import MingTaskSessionSetupMiddleware # noqa: E402 from allura.lib.custom_middleware import ContentSecurityPolicyMiddleware # noqa: E402 +from allura.lib.custom_middleware import StatusCodeRedirect # noqa: E402 from allura.lib import helpers as h # noqa: E402 from allura.lib.utils import configure_ming # noqa: E402 __all__ = ['make_app'] -def make_app(global_conf, full_stack=True, **app_conf): +# this is webapp entry point from setup.py +def make_app(global_conf: dict, **app_conf): override_root_module_name = app_conf.get('override_root', None) if override_root_module_name: # get an actual instance of it, like BasetestProjectRootController or TaskController @@ -80,7 +79,7 @@ def make_app(global_conf, full_stack=True, **app_conf): root = rootClass() else: root = None # will default to RootController in root.py - return _make_core_app(root, global_conf, full_stack, **app_conf) + return _make_core_app(root, global_conf, **app_conf) class BeakerPickleSerializerWithLatin1(PickleSerializer): @@ -89,7 +88,7 @@ class BeakerPickleSerializerWithLatin1(PickleSerializer): return pickle.loads(data_string, **{'encoding': 'latin1'} if six.PY3 else {}) -def _make_core_app(root, global_conf, full_stack=True, **app_conf): +def _make_core_app(root, global_conf: dict, **app_conf): """ Set allura up with the settings found in the PasteDeploy configuration file used. @@ -98,8 +97,6 @@ def _make_core_app(root, global_conf, full_stack=True, **app_conf): :param global_conf: The global settings for allura (those defined under the ``[DEFAULT]`` section). :type global_conf: dict - :param full_stack: Should the whole TG2 stack be set up? - :type full_stack: str or bool :return: The allura application with all the relevant middleware loaded. @@ -107,8 +104,6 @@ def _make_core_app(root, global_conf, full_stack=True, **app_conf): ``app_conf`` contains all the application-specific settings (those defined under ``[app:main]``. - - """ # Run all the initialization code here mimetypes.init([pkg_resources.resource_filename('allura', 'etc/mime.types')] + mimetypes.knownfiles) @@ -128,16 +123,7 @@ def _make_core_app(root, global_conf, full_stack=True, **app_conf): # Create base app base_config = ForgeConfig(root) - load_environment = base_config.make_load_environment() - - # Code adapted from tg.configuration, replacing the following lines: - # make_base_app = base_config.setup_tg_wsgi_app(load_environment) - # app = make_base_app(global_conf, full_stack=True, **app_conf) - - # Configure the TG environment - load_environment(global_conf, app_conf) - - app = tg.TGApp() + app = base_config.make_wsgi_app(global_conf, app_conf, wrap_app=None) for mw_ep in h.iter_entry_points('allura.middleware'): Middleware = mw_ep.load() @@ -204,16 +190,16 @@ def _make_core_app(root, global_conf, full_stack=True, **app_conf): # Converts exceptions to HTTP errors, shows traceback in debug mode app = DebuggedApplication(app, evalex=True) else: - app = ErrorMiddleware(app, config, **config['tg.errorware']) + app = ErrorMiddleware(app, config) app = SetRequestHostFromConfig(app, config) # Redirect some status codes to /error/document + handle_status_codes = [403, 404, 410] if asbool(config['debug']): - app = StatusCodeRedirect(app, base_config.handle_status_codes) + app = StatusCodeRedirect(app, handle_status_codes) else: - app = StatusCodeRedirect( - app, base_config.handle_status_codes + [500]) + app = StatusCodeRedirect(app, handle_status_codes + [500]) for mw_ep in h.iter_entry_points('allura.middleware'): Middleware = mw_ep.load() diff --git a/Allura/allura/lib/custom_middleware.py b/Allura/allura/lib/custom_middleware.py index 95c9a0122..e8fedd225 100644 --- a/Allura/allura/lib/custom_middleware.py +++ b/Allura/allura/lib/custom_middleware.py @@ -24,9 +24,8 @@ import pkg_resources from paste import fileapp from paste.deploy.converters import aslist, asbool from tg import tmpl_context as c -from tg.support.middlewares import _call_wsgi_application as call_wsgi_application from timermiddleware import Timer, TimerMiddleware -from webob import exc, Request +from webob import exc, Request, Response import pysolr import six from ming.odm import session @@ -154,7 +153,7 @@ class LoginRedirectMiddleware: self.app = app def __call__(self, environ, start_response): - status, headers, app_iter, exc_info = call_wsgi_application(self.app, environ) + status, headers, app_iter, exc_info = _call_wsgi_application(self.app, environ) is_api_request = environ.get('PATH_INFO', '').startswith('/rest/') if status[:3] == '401' and not is_api_request and not is_ajax(Request(environ)): login_url = tg.config.get('auth.login_url', '/auth/') @@ -520,3 +519,90 @@ class ContentSecurityPolicyMiddleware: report_rules.add(f'report-uri {report_uri}') resp.headers.add('Content-Security-Policy-Report-Only', '; '.join(report_rules)) return resp(environ, start_response) + + +""" +_call_wsgi_application & StatusCodeRedirect were originally part of TurboGears, but then removed from it. +They came from Pylons before that. + +TurboGears provides a ErrorPageApplicationWrapper alternative, but it is TG-specific and has behavior changes. +We'd have to enable in AppConfig with: + self['errorpage.enabled'] = True + self['errorpage.status_codes'] = [400, 401, 403, 404, 410] +And then lots of other changes: + `request.disable_error_pages()` instead of `request.environ['tg.status_code_redirect'] = True` + `request.disable_error_pages()` in jsonify() and some other places + `req = req.environ.get('tg.original_request') or req` in lots of middleware that uses the req *after* response + ... more +""" + + +def _call_wsgi_application(application, environ): + """ + Call the given WSGI application, returning ``(status_string, + headerlist, app_iter)`` + + Be sure to call ``app_iter.close()`` if it's there. + """ + captured = [] + output = [] + def _start_response(status, headers, exc_info=None): + captured[:] = [status, headers, exc_info] + return output.append + + app_iter = application(environ, _start_response) + if not captured or output: + try: + output.extend(app_iter) + finally: + if hasattr(app_iter, 'close'): + app_iter.close() + app_iter = output + return (captured[0], captured[1], app_iter, captured[2]) + + +class StatusCodeRedirect: + """Internally redirects a request based on status code + StatusCodeRedirect watches the response of the app it wraps. If the + response is an error code in the errors sequence passed the request + will be re-run with the path URL set to the path passed in. + This operation is non-recursive and the output of the second + request will be used no matter what it is. + Should an application wish to bypass the error response (ie, to + purposely return a 401), set + ``environ['tg.status_code_redirect'] = False`` in the application. + """ + def __init__(self, app, errors=(400, 401, 403, 404), + path='/error/document'): + """Initialize the ErrorRedirect + ``errors`` + A sequence (list, tuple) of error code integers that should + be caught. + ``path`` + The path to set for the next request down to the + application. + """ + self.app = app + self.error_path = path + + # Transform errors to str for comparison + self.errors = tuple([str(x) for x in errors]) + + def __call__(self, environ, start_response): + status, headers, app_iter, exc_info = _call_wsgi_application(self.app, environ) + if status[:3] in self.errors and \ + 'tg.status_code_redirect' not in environ and self.error_path: + # Create a response object + environ['tg.original_response'] = Response(status=status, headerlist=headers, app_iter=app_iter) + environ['tg.original_request'] = Request(environ) + + environ['pylons.original_response'] = environ['tg.original_response'] + environ['pylons.original_request'] = environ['tg.original_request'] + + # Create a new environ to avoid touching the original request data + new_environ = environ.copy() + new_environ['PATH_INFO'] = self.error_path + + newstatus, headers, app_iter, exc_info = _call_wsgi_application(self.app, new_environ) + start_response(status, headers, exc_info) + return app_iter diff --git a/Allura/allura/lib/helpers.py b/Allura/allura/lib/helpers.py index 721cf0d41..2bea1c07a 100644 --- a/Allura/allura/lib/helpers.py +++ b/Allura/allura/lib/helpers.py @@ -668,7 +668,7 @@ class fixed_attrs_proxy(proxy): @tg.expose(content_type='text/plain') def json_validation_error(controller, **kwargs): - exc = request.validation['exception'] + exc = request.validation.exception result = dict(status='Validation Error', errors={fld: str(err) for fld, err in exc.error_dict.items()}, value=exc.value, diff --git a/Allura/allura/tests/decorators.py b/Allura/allura/tests/decorators.py index 541f86d6c..8ef264f71 100644 --- a/Allura/allura/tests/decorators.py +++ b/Allura/allura/tests/decorators.py @@ -142,9 +142,9 @@ class patch_middleware_config: def __enter__(self): self._make_app = allura.config.middleware.make_app - def make_app(global_conf, full_stack=True, **app_conf): + def make_app(global_conf, **app_conf): app_conf.update(self.new_configs) - return self._make_app(global_conf, full_stack, **app_conf) + return self._make_app(global_conf, **app_conf) allura.config.middleware.make_app = make_app diff --git a/Allura/allura/tests/functional/test_root.py b/Allura/allura/tests/functional/test_root.py index 1feaa3b63..7ee73c600 100644 --- a/Allura/allura/tests/functional/test_root.py +++ b/Allura/allura/tests/functional/test_root.py @@ -27,9 +27,12 @@ Please read http://pythonpaste.org/webtest/ for more information. """ import os +import re from unittest import skipIf from tg import tmpl_context as c + +from allura.tests.decorators import patch_middleware_config from alluratest.tools import module_not_available from ming.odm.odmsession import ThreadLocalODMSession import mock @@ -239,3 +242,39 @@ class TestRootWithSSLPattern(TestController): assert '302 Found' not in r.text, r.text assert '301 Moved Permanently' not in r.text, r.text assert '/error/document' not in r.text, r.text + + +class TestErrorMiddleware(TestController): + def setup_method(self): + with patch_middleware_config({ + # this makes all the middleware get used (normally override_root=basetest_project_root makes it skip some) + 'override_root': None, + 'debug': 'false', + }): + super().setup_method() + + def test_404_error(self): + r = self.app.get('/this-is-a-404', status=404) + assert r.content_type == 'text/html' + r.mustcontain('404 Error has Occurred') # in error.html + r.mustcontain('This project is powered by') # in master template + + def test_error_nodebug(self): + with mock.patch.object(M.Project, 'ordered_mounts') as function_nbhd_controller_calls: + function_nbhd_controller_calls.side_effect = Exception('forced an error!') + r = self.app.get('/p/', + # suppress ErrorMiddleware from throwing the error, we want it to trap & return 500 page + extra_environ={'paste.throw_errors': False}, + status=500, + expect_errors=True, # because stderr gets error details printed to it + ) + assert r.status_code == 500 + assert r.content_type == 'text/html' + r.mustcontain('500 Error has Occurred') # in error.html + r.mustcontain('This project is powered by') # in master template + + # error logging from ErrorMiddleware + assert 'Exception: forced an error!' in r.errors + assert 'Allura/allura/controllers/project.py' in r.errors + assert 'CGI Variables' in r.errors + assert "PATH_INFO: '/p/'" in r.errors diff --git a/Allura/allura/websetup/__init__.py b/Allura/allura/websetup/__init__.py index 4677516ad..936dd149b 100644 --- a/Allura/allura/websetup/__init__.py +++ b/Allura/allura/websetup/__init__.py @@ -19,10 +19,10 @@ import logging -from allura.config.environment import load_environment - +from allura.config.app_cfg import base_config __all__ = ['setup_app'] + log = logging.getLogger(__name__) from .schema import setup_schema @@ -31,6 +31,8 @@ from . import bootstrap def setup_app(command, conf, vars): """Place any commands to setup allura here""" - load_environment(conf.global_conf, conf.local_conf) + conf = base_config.configure(conf.global_conf, conf.local_conf) + base_config.setup(conf) + setup_schema(command, conf, vars) bootstrap.bootstrap(command, conf, vars) diff --git a/Allura/development.ini b/Allura/development.ini index 2be945265..5be545809 100644 --- a/Allura/development.ini +++ b/Allura/development.ini @@ -71,7 +71,6 @@ port = 8080 ; [app:main] use = egg:Allura -full_stack = true ; Change this to your website's name site_name = Allura diff --git a/AlluraTest/alluratest/test_syntax.py b/AlluraTest/alluratest/test_syntax.py index bd2ea0701..a7cdc5490 100644 --- a/AlluraTest/alluratest/test_syntax.py +++ b/AlluraTest/alluratest/test_syntax.py @@ -62,7 +62,7 @@ def test_no_prints(): '/scripts/', 'ForgeSVN/setup.py', ] - if run(find_py + " | grep -v '" + "' | grep -v '".join(skips) + "' | xargs grep -v '^ *#' | grep 'print ' | grep -E -v '(pprint|#pragma: ?printok)' ") != 1: + if run(find_py + " | grep -v '" + "' | grep -v '".join(skips) + "' | xargs grep -v '^ *#' | egrep -n '\bprint\(' | grep -E -v '(pprint|#pragma: ?printok)' ") != 1: raise Exception("These should use logging instead of print") diff --git a/requirements.in b/requirements.in index 6183ebd8b..b5da04748 100644 --- a/requirements.in +++ b/requirements.in @@ -42,7 +42,7 @@ requests-oauthlib setproctitle six TimerMiddleware -TurboGears2==2.3.12 +TurboGears2 WebHelpers2 WebOb werkzeug diff --git a/requirements.txt b/requirements.txt index 82a8904f3..21b2be39b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -215,7 +215,7 @@ tomli==2.0.1 # via pytest translationstring==1.4 # via colander -turbogears2==2.3.12 +turbogears2==2.4.3 # via -r requirements.in typing-extensions==4.4.0 # via
