Prefer trailing slash in landing page URL when strict_slashes=True (#…

…1134)

pygeoapi/api.py

@@ -625,7 +625,6 @@ def __init__(self, config):
         """
 
         self.config = config
-        self.config['server']['url'] = self.config['server']['url'].rstrip('/')
         self.api_headers = get_api_rules(self.config).response_headers
         self.base_url = get_base_url(self.config)
         self.prefetcher = UrlPrefetcher()

pygeoapi/flask_app.py

@@ -122,7 +122,7 @@ def get_response(result: tuple):
     return response
 
 
-@BLUEPRINT.route('' if API_RULES.strict_slashes else '/')
+@BLUEPRINT.route('/')
 def landing_page():
     """
     OGC API landing page endpoint

pygeoapi/starlette_app.py

@@ -42,8 +42,11 @@
 from starlette.staticfiles import StaticFiles
 from starlette.applications import Starlette
 from starlette.requests import Request
+from starlette.datastructures import URL
 from starlette.types import ASGIApp, Scope, Send, Receive
-from starlette.responses import Response, JSONResponse, HTMLResponse
+from starlette.responses import (
+    Response, JSONResponse, HTMLResponse, RedirectResponse
+)
 import uvicorn
 
 from pygeoapi.api import API
@@ -485,15 +488,16 @@ async def __call__(self, scope: Scope,
         if scope['type'] == "http" and API_RULES.strict_slashes:
             path = scope['path']
             if path == self.prefix:
-                # If the root (landing page) is requested without
-                # a trailing slash, this should return a 200.
-                # However, because Starlette does not like an empty Route path,
-                # it will return a 404 instead.
-                # To prevent this, we will append a trailing slash here.
-                scope['path'] = f"{self.prefix}/"
-                scope['raw_path'] = scope['path'].encode()
-            elif path.endswith('/'):
-                # All other resource paths should not have trailing slashes
+                # If the root (landing page) is requested without a trailing
+                # slash, redirect to landing page with trailing slash.
+                # Starlette will otherwise throw a 404, as it does not like
+                # empty Route paths.
+                url = URL(scope=scope).replace(path=f"{path}/")
+                response = RedirectResponse(url)
+                await response(scope, receive, send)
+                return
+            elif path != f"{self.prefix}/" and path.endswith('/'):
+                # Resource paths should NOT have trailing slashes
                 response = Response(status_code=404)
                 await response(scope, receive, send)
                 return

tests/test_api.py

@@ -266,11 +266,16 @@ def test_apirules_active(config_with_rules, rules_api):
         # Test strict slashes
         response = flask_client.get(f'{flask_prefix}/conformance/')
         assert response.status_code == 404
+        # For the landing page ONLY, trailing slashes are actually preferred.
+        # See https://docs.opengeospatial.org/is/17-069r4/17-069r4.html#_api_landing_page  # noqa
+        # Omitting the trailing slash should lead to a redirect.
         response = flask_client.get(f'{flask_prefix}/')
-        assert response.status_code == 404
+        assert response.status_code == 200
+        response = flask_client.get(flask_prefix)
+        assert response.status_code in (307, 308)
 
         # Test links on landing page for correct URLs
-        response = flask_client.get(flask_prefix)
+        response = flask_client.get(flask_prefix, follow_redirects=True)
         assert response.status_code == 200
         assert response.is_json
         links = response.json['links']
@@ -291,11 +296,16 @@ def test_apirules_active(config_with_rules, rules_api):
         # Test strict slashes
         response = starlette_client.get(f'{starlette_prefix}/conformance/')
         assert response.status_code == 404
+        # For the landing page ONLY, trailing slashes are actually preferred.
+        # See https://docs.opengeospatial.org/is/17-069r4/17-069r4.html#_api_landing_page  # noqa
+        # Omitting the trailing slash should lead to a redirect.
         response = starlette_client.get(f'{starlette_prefix}/')
-        assert response.status_code == 404
+        assert response.status_code == 200
+        response = starlette_client.get(starlette_prefix)
+        assert response.status_code in (307, 308)
 
         # Test links on landing page for correct URLs
-        response = starlette_client.get(starlette_prefix)
+        response = starlette_client.get(starlette_prefix, follow_redirects=True)  # noqa
         assert response.status_code == 200
         links = response.json()['links']
         assert all(
@@ -311,32 +321,41 @@ def test_apirules_inactive(config, api_):
     flask_prefix = rules.get_url_prefix('flask')
     assert flask_prefix == ''
     with mock_flask('pygeoapi-test-config.yml') as flask_client:
-        # Test happy path
+        response = flask_client.get('')
+        assert response.status_code == 200
         response = flask_client.get('/conformance')
         assert response.status_code == 200
         assert 'X-API-Version' not in response.headers
+        assert response.request.url == \
+               flask_client.application.url_for('pygeoapi.conformance')
         response = flask_client.get('/static/img/pygeoapi.png')
         assert response.status_code == 200
 
-        # Test slash redirect
+        # Test trailing slashes
+        response = flask_client.get('/')
+        assert response.status_code == 200
         response = flask_client.get('/conformance/')
-        assert response.status_code != 404
+        assert response.status_code == 200
         assert 'X-API-Version' not in response.headers
 
     # Test Starlette
     starlette_prefix = rules.get_url_prefix('starlette')
     assert starlette_prefix == ''
     with mock_starlette('pygeoapi-test-config.yml') as starlette_client:
-        # Test happy path
+        response = starlette_client.get('')
+        assert response.status_code == 200
         response = starlette_client.get('/conformance')
         assert response.status_code == 200
         assert 'X-API-Version' not in response.headers
+        assert str(response.url) == f"{starlette_client.base_url}/conformance"
         response = starlette_client.get('/static/img/pygeoapi.png')
         assert response.status_code == 200
 
-        # Test slash redirect
-        response = starlette_client.get('/conformance/')
-        assert response.status_code != 404
+        # Test trailing slashes
+        response = starlette_client.get('/')
+        assert response.status_code == 200
+        response = starlette_client.get('/conformance/', follow_redirects=True)
+        assert response.status_code == 200
         assert 'X-API-Version' not in response.headers
 
 

tests/util.py

@@ -34,8 +34,6 @@
 from importlib import reload
 from contextlib import contextmanager
 
-from pygeoapi.util import get_base_url
-
 from flask.testing import FlaskClient
 from starlette.testclient import TestClient as StarletteClient
 from werkzeug.test import create_environ
@@ -83,6 +81,8 @@ def mock_request(params: dict = None, data=None, **headers) -> Request:
 def mock_flask(config_file: str = 'pygeoapi-test-config.yml', **kwargs) -> FlaskClient:  # noqa
     """
     Mocks a Flask client so we can test the API routing with applied API rules.
+    Does not follow redirects by default. Set `follow_redirects=True` option
+    on individual requests to enable.
 
     :param config_file: Optional configuration YAML file to use.
                         If not set, the default test configuration is used.
@@ -100,7 +100,7 @@ def mock_flask(config_file: str = 'pygeoapi-test-config.yml', **kwargs) -> Flask
         reload(flask_app)
 
         # Set server root path
-        url_parts = urlsplit(get_base_url(flask_app.CONFIG))
+        url_parts = urlsplit(flask_app.CONFIG['server']['url'])
         app_root = url_parts.path.rstrip('/') or '/'
         flask_app.APP.config['SERVER_NAME'] = url_parts.netloc
         flask_app.APP.config['APPLICATION_ROOT'] = app_root
@@ -128,6 +128,8 @@ def mock_starlette(config_file: str = 'pygeoapi-test-config.yml', **kwargs) -> S
     """
     Mocks a Starlette client so we can test the API routing with applied
     API rules.
+    Does not follow redirects by default. Set `follow_redirects=True` option
+    on individual requests to enable.
 
     :param config_file: Optional configuration YAML file to use.
                         If not set, the default test configuration is used.
@@ -145,7 +147,7 @@ def mock_starlette(config_file: str = 'pygeoapi-test-config.yml', **kwargs) -> S
         reload(starlette_app)
 
         # Get server root path
-        base_url = get_base_url(starlette_app.CONFIG)
+        base_url = starlette_app.CONFIG['server']['url'].rstrip('/')
         root_path = urlsplit(base_url).path.rstrip('/') or ''
 
         # Create and return test client
@@ -157,6 +159,8 @@ def mock_starlette(config_file: str = 'pygeoapi-test-config.yml', **kwargs) -> S
             root_path=root_path,
             **kwargs
         )
+        # Override follow_redirects so behavior is the same as Flask mock
+        client.follow_redirects = False
         yield client
 
     finally: