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()
@@ -649,6 +648,10 @@ def __init__(self, config):
 
         setup_logger(self.config['logging'])
 
+        # Create config clone for HTML templating with modified base URL
+        self.tpl_config = deepcopy(self.config)
+        self.tpl_config['server']['url'] = self.base_url
+
         # TODO: add as decorator
         if 'manager' in self.config['server']:
             manager_def = self.config['server']['manager']
@@ -756,8 +759,8 @@ def landing_page(self,
                                         'type', 'stac-collection'):
                 fcm['stac'] = True
 
-            content = render_j2_template(self.config, 'landing_page.html', fcm,
-                                         request.locale)
+            content = render_j2_template(self.tpl_config, 'landing_page.html',
+                                         fcm, request.locale)
             return headers, HTTPStatus.OK, content
 
         if request.format == F_JSONLD:
@@ -793,7 +796,7 @@ def openapi(self, request: Union[APIRequest, Any],
             data = {
                 'openapi-document-path': path
             }
-            content = render_j2_template(self.config, template, data,
+            content = render_j2_template(self.tpl_config, template, data,
                                          request.locale)
             return headers, HTTPStatus.OK, content
 
@@ -835,7 +838,7 @@ def conformance(self,
 
         headers = request.get_response_headers(**self.api_headers)
         if request.format == F_HTML:  # render
-            content = render_j2_template(self.config, 'conformance.html',
+            content = render_j2_template(self.tpl_config, 'conformance.html',
                                          conformance, request.locale)
             return headers, HTTPStatus.OK, content
 
@@ -1206,11 +1209,11 @@ def describe_collections(self, request: Union[APIRequest, Any],
         if request.format == F_HTML:  # render
             fcm['collections_path'] = self.get_collections_url()
             if dataset is not None:
-                content = render_j2_template(self.config,
+                content = render_j2_template(self.tpl_config,
                                              'collections/collection.html',
                                              fcm, request.locale)
             else:
-                content = render_j2_template(self.config,
+                content = render_j2_template(self.tpl_config,
                                              'collections/index.html', fcm,
                                              request.locale)
 
@@ -1311,7 +1314,7 @@ def get_collection_queryables(self, request: Union[APIRequest, Any],
 
             queryables['collections_path'] = self.get_collections_url()
 
-            content = render_j2_template(self.config,
+            content = render_j2_template(self.tpl_config,
                                          'collections/queryables.html',
                                          queryables, request.locale)
 
@@ -1654,7 +1657,7 @@ def get_collection_items(
                                                         request.locale)
                 # If title exists, use it as id in html templates
                 content['id_field'] = content['title_field']
-            content = render_j2_template(self.config,
+            content = render_j2_template(self.tpl_config,
                                          'collections/items/index.html',
                                          content, request.locale)
             return headers, HTTPStatus.OK, content
@@ -2245,7 +2248,7 @@ def get_collection_item(self, request: Union[APIRequest, Any],
                                                         request.locale)
             content['collections_path'] = self.get_collections_url()
 
-            content = render_j2_template(self.config,
+            content = render_j2_template(self.tpl_config,
                                          'collections/items/item.html',
                                          content, request.locale)
             return headers, HTTPStatus.OK, content
@@ -2461,7 +2464,7 @@ def get_collection_coverage_domainset(
                 self.config['resources'][dataset]['title'],
                 self.default_locale)
             data['collections_path'] = self.get_collections_url()
-            content = render_j2_template(self.config,
+            content = render_j2_template(self.tpl_config,
                                          'collections/coverage/domainset.html',
                                          data, self.default_locale)
             return headers, HTTPStatus.OK, content
@@ -2518,7 +2521,7 @@ def get_collection_coverage_rangetype(
                 self.config['resources'][dataset]['title'],
                 self.default_locale)
             data['collections_path'] = self.get_collections_url()
-            content = render_j2_template(self.config,
+            content = render_j2_template(self.tpl_config,
                                          'collections/coverage/rangetype.html',
                                          data, self.default_locale)
             return headers, HTTPStatus.OK, content
@@ -2643,7 +2646,7 @@ def get_collection_tiles(self, request: Union[APIRequest, Any],
             tiles['maxzoom'] = p.options['zoom']['max']
             tiles['collections_path'] = self.get_collections_url()
 
-            content = render_j2_template(self.config,
+            content = render_j2_template(self.tpl_config,
                                          'collections/tiles/index.html', tiles,
                                          request.locale)
 
@@ -2817,7 +2820,7 @@ def get_collection_tiles_metadata(
             metadata['format'] = metadata_format.value
             metadata['collections_path'] = self.get_collections_url()
 
-            content = render_j2_template(self.config,
+            content = render_j2_template(self.tpl_config,
                                          'collections/tiles/metadata.html',
                                          metadata, request.locale)
 
@@ -3239,11 +3242,11 @@ def describe_processes(self, request: Union[APIRequest, Any],
 
         if request.format == F_HTML:  # render
             if process is not None:
-                response = render_j2_template(self.config,
+                response = render_j2_template(self.tpl_config,
                                               'processes/process.html',
                                               response, request.locale)
             else:
-                response = render_j2_template(self.config,
+                response = render_j2_template(self.tpl_config,
                                               'processes/index.html', response,
                                               request.locale)
 
@@ -3345,7 +3348,7 @@ def get_jobs(self, request: Union[APIRequest, Any],
                 'jobs': serialized_jobs,
                 'now': datetime.now(timezone.utc).strftime(DATETIME_FORMAT)
             }
-            response = render_j2_template(self.config, j2_template, data,
+            response = render_j2_template(self.tpl_config, j2_template, data,
                                           request.locale)
             return headers, HTTPStatus.OK, response
 
@@ -3723,7 +3726,7 @@ def get_collection_edr_query(
                 'NoApplicableCode', str(err))
 
         if request.format == F_HTML:  # render
-            content = render_j2_template(self.config,
+            content = render_j2_template(self.tpl_config,
                                          'collections/edr/query.html', data,
                                          self.default_locale)
         else:
@@ -3781,7 +3784,8 @@ def get_stac_root(
             })
 
         if request.format == F_HTML:  # render
-            content = render_j2_template(self.config, 'stac/collection.html',
+            content = render_j2_template(self.tpl_config,
+                                         'stac/collection.html',
                                          content, request.locale)
             return headers, HTTPStatus.OK, content
 
@@ -3865,11 +3869,11 @@ def get_stac_path(self, request: Union[APIRequest, Any],
             if request.format == F_HTML:  # render
                 content['path'] = path
                 if 'assets' in content:  # item view
-                    content = render_j2_template(self.config,
+                    content = render_j2_template(self.tpl_config,
                                                  'stac/item.html',
                                                  content, request.locale)
                 else:
-                    content = render_j2_template(self.config,
+                    content = render_j2_template(self.tpl_config,
                                                  'stac/catalog.html',
                                                  content, request.locale)
 

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
@@ -550,6 +554,15 @@ async def __call__(self, scope: Scope,
     ]
 )
 
+if url_prefix:
+    # If a URL prefix is in effect, Flask allows the static resource URLs
+    # to be written both with or without that prefix (200 in both cases).
+    # Starlette does not allow this, so for consistency we'll add a static
+    # mount here WITHOUT the URL prefix (due to router order).
+    APP.mount(
+        '/static', StaticFiles(directory=STATIC_DIR),
+    )
+
 # If API rules require strict slashes, do not redirect
 if API_RULES.strict_slashes:
     APP.router.redirect_slashes = False

tests/test_api.py

@@ -262,15 +262,23 @@ def test_apirules_active(config_with_rules, rules_api):
                flask_client.application.url_for('pygeoapi.conformance')
         response = flask_client.get(f'{flask_prefix}/static/img/pygeoapi.png')
         assert response.status_code == 200
+        # Test that static resources also work without URL prefix
+        response = flask_client.get('/static/img/pygeoapi.png')
+        assert response.status_code == 200
 
         # 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']
@@ -287,15 +295,23 @@ def test_apirules_active(config_with_rules, rules_api):
         assert response.headers['X-API-Version'] == __version__
         response = starlette_client.get(f'{starlette_prefix}/static/img/pygeoapi.png')  # noqa
         assert response.status_code == 200
+        # Test that static resources also work without URL prefix
+        response = starlette_client.get('/static/img/pygeoapi.png')
+        assert response.status_code == 200
 
         # 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 +327,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: