From 8cec2010c0564bbd6e1f9cd8881404f70178247f Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Mon, 19 Feb 2018 21:20:03 +0100 Subject: [PATCH 1/2] Do not enable subdomain matching by default Updated tests for new subdomain matching Added a test to validate matching behavior --- CHANGES.rst | 2 ++ flask/app.py | 16 +++++++++++++++- tests/test_basic.py | 33 +++++++++++++++++++++++++++++---- tests/test_testing.py | 8 ++++++-- 4 files changed, 52 insertions(+), 7 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 84fa436f..8b0a945e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -139,6 +139,8 @@ unreleased attribute on the session cookie. (`#2607`_) - Added :meth:`~flask.Flask.test_cli_runner` to create a Click runner that can invoke Flask CLI commands for testing. (`#2636`_) +- Subdomain matching is disabled by default now. It can be turned on by + passing ``subdomain_matching=True`` to the Flask constructor. .. _pallets/meta#24: https://github.com/pallets/meta/issues/24 .. _#1421: https://github.com/pallets/flask/issues/1421 diff --git a/flask/app.py b/flask/app.py index 7c53a583..70a0c65b 100644 --- a/flask/app.py +++ b/flask/app.py @@ -126,6 +126,11 @@ class Flask(_PackageBoundObject): .. versionadded:: 0.13 The `host_matching` and `static_host` parameters were added. + .. versionadded:: 1.0 + The `subdomain_matching` parameter was added. Subdomain matching + needs to be enabled manually now. Setting `SERVER_NAME` does not + implicitly enable it. + :param import_name: the name of the application package :param static_url_path: can be used to specify a different path for the static files on the web. Defaults to the name @@ -347,6 +352,7 @@ class Flask(_PackageBoundObject): static_folder='static', static_host=None, host_matching=False, + subdomain_matching=False, template_folder='templates', instance_path=None, instance_relative_config=False, @@ -530,6 +536,7 @@ class Flask(_PackageBoundObject): self.url_map = Map() self.url_map.host_matching = host_matching + self.subdomain_matching = subdomain_matching # tracks internally if the application already handled at least one # request. @@ -1988,8 +1995,15 @@ class Flask(_PackageBoundObject): URL adapter is created for the application context. """ if request is not None: - return self.url_map.bind_to_environ(request.environ, + rv = self.url_map.bind_to_environ(request.environ, server_name=self.config['SERVER_NAME']) + # If subdomain matching is not enabled (which is the default + # we put back the default subdomain in all cases. This really + # should be the default in Werkzeug but it currently does not + # have that feature. + if not self.subdomain_matching: + rv.subdomain = self.url_map.default_subdomain + return rv # We need at the very least the server name to be set for this # to work. if self.config['SERVER_NAME'] is not None: diff --git a/tests/test_basic.py b/tests/test_basic.py index a054ae39..ecf3ee71 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -1429,10 +1429,12 @@ def test_request_locals(): assert not flask.g -def test_test_app_proper_environ(app, client): +def test_test_app_proper_environ(): + app = flask.Flask(__name__, subdomain_matching=True) app.config.update( SERVER_NAME='localhost.localdomain:5000' ) + client = app.test_client() @app.route('/') def index(): @@ -1783,8 +1785,10 @@ def test_g_iteration_protocol(app_ctx): assert sorted(flask.g) == ['bar', 'foo'] -def test_subdomain_basic_support(app, client): +def test_subdomain_basic_support(): + app = flask.Flask(__name__, subdomain_matching=True) app.config['SERVER_NAME'] = 'localhost.localdomain' + client = app.test_client() @app.route('/') def normal_index(): @@ -1801,7 +1805,9 @@ def test_subdomain_basic_support(app, client): assert rv.data == b'test index' -def test_subdomain_matching(app, client): +def test_subdomain_matching(): + app = flask.Flask(__name__, subdomain_matching=True) + client = app.test_client() app.config['SERVER_NAME'] = 'localhost.localdomain' @app.route('/', subdomain='') @@ -1812,8 +1818,10 @@ def test_subdomain_matching(app, client): assert rv.data == b'index for mitsuhiko' -def test_subdomain_matching_with_ports(app, client): +def test_subdomain_matching_with_ports(): + app = flask.Flask(__name__, subdomain_matching=True) app.config['SERVER_NAME'] = 'localhost.localdomain:3000' + client = app.test_client() @app.route('/', subdomain='') def index(user): @@ -1823,6 +1831,23 @@ def test_subdomain_matching_with_ports(app, client): assert rv.data == b'index for mitsuhiko' +def test_subdomain_matching_behavior(): + for matching in False, True: + app = flask.Flask(__name__, subdomain_matching=matching) + app.config['SERVER_NAME'] = 'localhost.localdomain:3000' + client = app.test_client() + + @app.route('/') + def index(): + return 'matched without subdomain' + + rv = client.get('/', 'http://127.0.0.1:3000/') + if matching: + assert rv.status_code == 404 + else: + assert rv.data == b'matched without subdomain' + + def test_multi_route_rules(app, client): @app.route('/') @app.route('//') diff --git a/tests/test_testing.py b/tests/test_testing.py index b0619d2c..14c66324 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -114,9 +114,11 @@ def test_path_is_url(app): assert eb.path == '/' -def test_blueprint_with_subdomain(app, client): +def test_blueprint_with_subdomain(): + app = flask.Flask(__name__, subdomain_matching=True) app.config['SERVER_NAME'] = 'example.com:1234' app.config['APPLICATION_ROOT'] = '/foo' + client = app.test_client() bp = flask.Blueprint('company', __name__, subdomain='xxx') @@ -304,8 +306,10 @@ def test_json_request_and_response(app, client): assert rv.get_json() == json_data -def test_subdomain(app, client): +def test_subdomain(): + app = flask.Flask(__name__, subdomain_matching=True) app.config['SERVER_NAME'] = 'example.com' + client = app.test_client() @app.route('/', subdomain='') def view(company_id): From 82f0d120deb9ea52237d95a14a4a619b01d06f80 Mon Sep 17 00:00:00 2001 From: David Lord Date: Fri, 23 Feb 2018 07:51:34 -0800 Subject: [PATCH 2/2] use subdomain arg in url_map.bind_to_environ rename new subdomain test, parametrize test allowing subdomains as well as ips add subdomain_matching param to docs add some references to docs add version changed to create_url_adapter --- CHANGES.rst | 7 +++++-- docs/config.rst | 15 +++++++------- flask/app.py | 50 +++++++++++++++++++++++++-------------------- tests/test_basic.py | 28 +++++++++++++------------ 4 files changed, 56 insertions(+), 44 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 8b0a945e..6a363149 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -139,8 +139,10 @@ unreleased attribute on the session cookie. (`#2607`_) - Added :meth:`~flask.Flask.test_cli_runner` to create a Click runner that can invoke Flask CLI commands for testing. (`#2636`_) -- Subdomain matching is disabled by default now. It can be turned on by - passing ``subdomain_matching=True`` to the Flask constructor. +- Subdomain matching is disabled by default and setting + :data:`SERVER_NAME` does not implicily enable it. It can be enabled by + passing ``subdomain_matching=True`` to the ``Flask`` constructor. + (`#2635`_) .. _pallets/meta#24: https://github.com/pallets/meta/issues/24 .. _#1421: https://github.com/pallets/flask/issues/1421 @@ -183,6 +185,7 @@ unreleased .. _#2606: https://github.com/pallets/flask/pull/2606 .. _#2607: https://github.com/pallets/flask/pull/2607 .. _#2636: https://github.com/pallets/flask/pull/2636 +.. _#2635: https://github.com/pallets/flask/pull/2635 Version 0.12.2 diff --git a/docs/config.rst b/docs/config.rst index 2e2833f9..c496ac00 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -181,8 +181,8 @@ The following configuration values are used internally by Flask: .. py:data:: SESSION_COOKIE_DOMAIN The domain match rule that the session cookie will be valid for. If not - set, the cookie will be valid for all subdomains of ``SERVER_NAME``. If - ``False``, the cookie's domain will not be set. + set, the cookie will be valid for all subdomains of :data:`SERVER_NAME`. + If ``False``, the cookie's domain will not be set. Default: ``None`` @@ -257,13 +257,14 @@ The following configuration values are used internally by Flask: .. py:data:: SERVER_NAME - Inform the application what host and port it is bound to. Required for - subdomain route matching support. + Inform the application what host and port it is bound to. Required + for subdomain route matching support. If set, will be used for the session cookie domain if - ``SESSION_COOKIE_DOMAIN`` is not set. Modern web browsers will not allow - setting cookies for domains without a dot. To use a domain locally, - add any names that should route to the app to your ``hosts`` file. :: + :data:`SESSION_COOKIE_DOMAIN` is not set. Modern web browsers will + not allow setting cookies for domains without a dot. To use a domain + locally, add any names that should route to the app to your + ``hosts`` file. :: 127.0.0.1 localhost.dev diff --git a/flask/app.py b/flask/app.py index 70a0c65b..89be4dc5 100644 --- a/flask/app.py +++ b/flask/app.py @@ -123,13 +123,13 @@ class Flask(_PackageBoundObject): .. versionadded:: 0.11 The `root_path` parameter was added. - .. versionadded:: 0.13 - The `host_matching` and `static_host` parameters were added. + .. versionadded:: 1.0 + The ``host_matching`` and ``static_host`` parameters were added. .. versionadded:: 1.0 - The `subdomain_matching` parameter was added. Subdomain matching - needs to be enabled manually now. Setting `SERVER_NAME` does not - implicitly enable it. + The ``subdomain_matching`` parameter was added. Subdomain + matching needs to be enabled manually now. Setting + :data:`SERVER_NAME` does not implicitly enable it. :param import_name: the name of the application package :param static_url_path: can be used to specify a different path for the @@ -138,11 +138,13 @@ class Flask(_PackageBoundObject): :param static_folder: the folder with static files that should be served at `static_url_path`. Defaults to the ``'static'`` folder in the root path of the application. - :param host_matching: sets the app's ``url_map.host_matching`` to the given - value. Defaults to False. - :param static_host: the host to use when adding the static route. Defaults - to None. Required when using ``host_matching=True`` - with a ``static_folder`` configured. + :param static_host: the host to use when adding the static route. + Defaults to None. Required when using ``host_matching=True`` + with a ``static_folder`` configured. + :param host_matching: set ``url_map.host_matching`` attribute. + Defaults to False. + :param subdomain_matching: consider the subdomain relative to + :data:`SERVER_NAME` when matching routes. Defaults to False. :param template_folder: the folder that contains the templates that should be used by the application. Defaults to ``'templates'`` folder in the root path of the @@ -1984,26 +1986,30 @@ class Flask(_PackageBoundObject): return rv def create_url_adapter(self, request): - """Creates a URL adapter for the given request. The URL adapter - is created at a point where the request context is not yet set up - so the request is passed explicitly. + """Creates a URL adapter for the given request. The URL adapter + is created at a point where the request context is not yet set + up so the request is passed explicitly. .. versionadded:: 0.6 .. versionchanged:: 0.9 This can now also be called without a request object when the URL adapter is created for the application context. + + .. versionchanged:: 1.0 + :data:`SERVER_NAME` no longer implicitly enables subdomain + matching. Use :attr:`subdomain_matching` instead. """ if request is not None: - rv = self.url_map.bind_to_environ(request.environ, - server_name=self.config['SERVER_NAME']) - # If subdomain matching is not enabled (which is the default - # we put back the default subdomain in all cases. This really - # should be the default in Werkzeug but it currently does not - # have that feature. - if not self.subdomain_matching: - rv.subdomain = self.url_map.default_subdomain - return rv + # If subdomain matching is disabled (the default), use the + # default subdomain in all cases. This should be the default + # in Werkzeug but it currently does not have that feature. + subdomain = ((self.url_map.default_subdomain or None) + if not self.subdomain_matching else None) + return self.url_map.bind_to_environ( + request.environ, + server_name=self.config['SERVER_NAME'], + subdomain=subdomain) # We need at the very least the server name to be set for this # to work. if self.config['SERVER_NAME'] is not None: diff --git a/tests/test_basic.py b/tests/test_basic.py index ecf3ee71..66e0d907 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -1831,21 +1831,23 @@ def test_subdomain_matching_with_ports(): assert rv.data == b'index for mitsuhiko' -def test_subdomain_matching_behavior(): - for matching in False, True: - app = flask.Flask(__name__, subdomain_matching=matching) - app.config['SERVER_NAME'] = 'localhost.localdomain:3000' - client = app.test_client() +@pytest.mark.parametrize('matching', (False, True)) +def test_subdomain_matching_other_name(matching): + app = flask.Flask(__name__, subdomain_matching=matching) + app.config['SERVER_NAME'] = 'localhost.localdomain:3000' + client = app.test_client() - @app.route('/') - def index(): - return 'matched without subdomain' + @app.route('/') + def index(): + return '', 204 - rv = client.get('/', 'http://127.0.0.1:3000/') - if matching: - assert rv.status_code == 404 - else: - assert rv.data == b'matched without subdomain' + # ip address can't match name + rv = client.get('/', 'http://127.0.0.1:3000/') + assert rv.status_code == 404 if matching else 204 + + # allow all subdomains if matching is disabled + rv = client.get('/', 'http://www.localhost.localdomain:3000/') + assert rv.status_code == 404 if matching else 204 def test_multi_route_rules(app, client):