From bb78691e70e9519459017d66e5c110ccaa176541 Mon Sep 17 00:00:00 2001 From: David Bibb Date: Fri, 10 Aug 2018 09:35:13 -0600 Subject: [PATCH] Add option to remove slash to `normalize_path_middleware` (#3173) --- CHANGES/3173.doc | 1 + CHANGES/3173.feature | 1 + CONTRIBUTORS.txt | 1 + aiohttp/web_middlewares.py | 40 +++++++++++++++------ docs/web_reference.rst | 48 +++++++++++++++++--------- tests/test_web_middleware.py | 67 ++++++++++++++++++++++++++++++++++++ 6 files changed, 130 insertions(+), 28 deletions(-) create mode 100644 CHANGES/3173.doc create mode 100644 CHANGES/3173.feature diff --git a/CHANGES/3173.doc b/CHANGES/3173.doc new file mode 100644 index 0000000000..c826ca2438 --- /dev/null +++ b/CHANGES/3173.doc @@ -0,0 +1 @@ +Updated ``normalize_path_middleware`` docs to show new remove_slash functionality. diff --git a/CHANGES/3173.feature b/CHANGES/3173.feature new file mode 100644 index 0000000000..1b95038abf --- /dev/null +++ b/CHANGES/3173.feature @@ -0,0 +1 @@ +Added a ``remove_slash`` option to the ``normalize_path_middleware`` factory. diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index bd7c52c824..147d8cb996 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -57,6 +57,7 @@ Dan Xu Daniel García Daniel Nelson Danny Song +David Bibb David Michael Brown Denilson Amorim Denis Matiychuk diff --git a/aiohttp/web_middlewares.py b/aiohttp/web_middlewares.py index 57b735c1c7..f762f5b8bf 100644 --- a/aiohttp/web_middlewares.py +++ b/aiohttp/web_middlewares.py @@ -28,28 +28,41 @@ def middleware(f): def normalize_path_middleware( - *, append_slash=True, merge_slashes=True, - redirect_class=HTTPMovedPermanently): + *, append_slash=True, remove_slash=False, + merge_slashes=True, redirect_class=HTTPMovedPermanently): """ - Middleware that normalizes the path of a request. By normalizing - it means: + Middleware factory which produces a middleware that normalizes + the path of a request. By normalizing it means: - - Add a trailing slash to the path. + - Add or remove a trailing slash to the path. - Double slashes are replaced by one. The middleware returns as soon as it finds a path that resolves - correctly. The order if all enable is 1) merge_slashes, 2) append_slash - and 3) both merge_slashes and append_slash. If the path resolves with - at least one of those conditions, it will redirect to the new path. + correctly. The order if both merge and append/remove are enabled is + 1) merge slashes + 2) append/remove slash + 3) both merge slashes and append/remove slash. + If the path resolves with at least one of those conditions, it will + redirect to the new path. - If append_slash is True append slash when needed. If a resource is - defined with trailing slash and the request comes without it, it will - append it automatically. + Only one of `append_slash` and `remove_slash` can be enabled. If both + are `True` the factory will raise an assertion error + + If `append_slash` is `True` the middleware will append a slash when + needed. If a resource is defined with trailing slash and the request + comes without it, it will append it automatically. + + If `remove_slash` is `True`, `append_slash` must be `False`. When enabled + the middleware will remove trailing slashes and redirect if the resource + is defined If merge_slashes is True, merge multiple consecutive slashes in the path into one. """ + correct_configuration = not (append_slash and remove_slash) + assert correct_configuration, "Cannot both remove and append slash" + @middleware async def impl(request, handler): if isinstance(request.match_info.route, SystemRoute): @@ -65,9 +78,14 @@ async def impl(request, handler): paths_to_check.append(re.sub('//+', '/', path)) if append_slash and not request.path.endswith('/'): paths_to_check.append(path + '/') + if remove_slash and request.path.endswith('/'): + paths_to_check.append(path[:-1]) if merge_slashes and append_slash: paths_to_check.append( re.sub('//+', '/', path + '/')) + if merge_slashes and remove_slash: + merged_slashes = re.sub('//+', '/', path) + paths_to_check.append(merged_slashes[:-1]) for path in paths_to_check: resolves, request = await _check_request_resolves( diff --git a/docs/web_reference.rst b/docs/web_reference.rst index ae0d1cee75..ee59015a84 100644 --- a/docs/web_reference.rst +++ b/docs/web_reference.rst @@ -2762,27 +2762,41 @@ Normalize path middleware ^^^^^^^^^^^^^^^^^^^^^^^^^ .. function:: normalize_path_middleware(*, \ - append_slash=True, merge_slashes=True) + append_slash=True, \ + remove_slash=False, \ + merge_slashes=True, \ + redirect_class=HTTPMovedPermanently) - Middleware factory which produces a middleware that normalizes - the path of a request. By normalizing it means: + Middleware factory which produces a middleware that normalizes + the path of a request. By normalizing it means: - - Add a trailing slash to the path. - - Double slashes are replaced by one. + - Add or remove a trailing slash to the path. + - Double slashes are replaced by one. - The middleware returns as soon as it finds a path that resolves - correctly. The order if all enabled is: + The middleware returns as soon as it finds a path that resolves + correctly. The order if both merge and append/remove are enabled is: - 1. *merge_slashes* - 2. *append_slash* - 3. both *merge_slashes* and *append_slash* + 1. *merge_slashes* + 2. *append_slash* or *remove_slash* + 3. both *merge_slashes* and *append_slash* or *remove_slash* - If the path resolves with at least one of those conditions, it will - redirect to the new path. + If the path resolves with at least one of those conditions, it will + redirect to the new path. - If *append_slash* is ``True`` append slash when needed. If a resource is - defined with trailing slash and the request comes without it, it will - append it automatically. + Only one of *append_slash* and *remove_slash* can be enabled. If both are + ``True`` the factory will raise an ``AssertionError`` - If *merge_slashes* is ``True``, merge multiple consecutive slashes in the - path into one. + If *append_slash* is ``True`` the middleware will append a slash when + needed. If a resource is defined with trailing slash and the request + comes without it, it will append it automatically. + + If *remove_slash* is ``True``, *append_slash* must be ``False``. When enabled + the middleware will remove trailing slashes and redirect if the resource is + defined. + + If *merge_slashes* is ``True``, merge multiple consecutive slashes in the + path into one. + + .. versionadded:: 3.4 + + Support for *remove_slash* diff --git a/tests/test_web_middleware.py b/tests/test_web_middleware.py index b7ff59c22b..e107cc17b1 100644 --- a/tests/test_web_middleware.py +++ b/tests/test_web_middleware.py @@ -116,6 +116,27 @@ async def test_add_trailing_when_necessary( resp = await client.get(path) assert resp.status == status + @pytest.mark.parametrize("path, status", [ + ('/resource1', 200), + ('/resource1/', 200), + ('/resource2', 404), + ('/resource2/', 200), + ('/resource1?p1=1&p2=2', 200), + ('/resource1/?p1=1&p2=2', 200), + ('/resource2?p1=1&p2=2', 404), + ('/resource2/?p1=1&p2=2', 200), + ('/resource2/a/b%2Fc', 404), + ('/resource2/a/b%2Fc/', 200) + ]) + async def test_remove_trailing_when_necessary(self, path, status, cli): + extra_middlewares = [ + web.normalize_path_middleware( + append_slash=False, remove_slash=True, merge_slashes=False)] + client = await cli(extra_middlewares) + + resp = await client.get(path) + assert resp.status == status + @pytest.mark.parametrize("path, status", [ ('/resource1', 200), ('/resource1/', 404), @@ -200,6 +221,52 @@ async def test_append_and_merge_slash(self, path, status, cli): resp = await client.get(path) assert resp.status == status + @pytest.mark.parametrize("path, status", [ + ('/resource1/a/b', 200), + ('/resource1/a/b/', 200), + ('//resource2//a//b', 404), + ('//resource2//a//b/', 200), + ('///resource1//a//b', 200), + ('///resource1//a//b/', 200), + ('/////resource1/a///b', 200), + ('/////resource1/a///b/', 200), + ('/////resource1/a///b///', 200), + ('/resource2/a/b', 404), + ('//resource2//a//b', 404), + ('//resource2//a//b/', 200), + ('///resource2//a//b', 404), + ('///resource2//a//b/', 200), + ('/////resource2/a///b', 404), + ('/////resource2/a///b/', 200), + ('/resource1/a/b?p=1', 200), + ('/resource1/a/b/?p=1', 200), + ('//resource2//a//b?p=1', 404), + ('//resource2//a//b/?p=1', 200), + ('///resource1//a//b?p=1', 200), + ('///resource1//a//b/?p=1', 200), + ('/////resource1/a///b?p=1', 200), + ('/////resource1/a///b/?p=1', 200), + ('/resource2/a/b?p=1', 404), + ('//resource2//a//b?p=1', 404), + ('//resource2//a//b/?p=1', 200), + ('///resource2//a//b?p=1', 404), + ('///resource2//a//b/?p=1', 200), + ('/////resource2/a///b?p=1', 404), + ('/////resource2/a///b/?p=1', 200) + ]) + async def test_remove_and_merge_slash(self, path, status, cli): + extra_middlewares = [ + web.normalize_path_middleware( + append_slash=False, remove_slash=True)] + + client = await cli(extra_middlewares) + resp = await client.get(path) + assert resp.status == status + + async def test_cannot_remove_and_add_slash(self): + with pytest.raises(AssertionError): + web.normalize_path_middleware(append_slash=True, remove_slash=True) + async def test_old_style_middleware(loop, aiohttp_client): async def handler(request):