Add option to remove slash to normalize_path_middleware (#3173)

CHANGES/3173.doc

@@ -0,0 +1 @@
+Updated ``normalize_path_middleware`` docs to show new remove_slash functionality. 

CHANGES/3173.feature

@@ -0,0 +1 @@
+Added a ``remove_slash`` option to the ``normalize_path_middleware`` factory.

CONTRIBUTORS.txt

@@ -57,6 +57,7 @@ Dan Xu
 Daniel García
 Daniel Nelson
 Danny Song
+David Bibb
 David Michael Brown
 Denilson Amorim
 Denis Matiychuk

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(

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*

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):