Repository: biopragmatics/bioregistry

Issue:
Can't construct reference page if slash in identifier
              I believe this is still/again an issue: look at e.g. the registry page for [CTRI](https://bioregistry.io/registry/ctri) (India Clinical Trials Registry), and click on the example CURIE: https://bioregistry.io/reference/ctri:CTRI/2023/04/052053, at least for me I get an internal server error instead of it resolving.

_Originally posted by @kkaris in https://github.com/biopragmatics/bioregistry/issues/31#issuecomment-2030201710_
            
It looks like the issue is inside the template, where starlette isn't happy if there's a slash inside any of the parameters in constructing a path

```python-stacktrace
2024-04-02 08:54:05 ERROR    Exception on /reference/ctri:CTRI/2023/04/052053 [GET]
Traceback (most recent call last):
  File "/Users/cthoyt/.virtualenvs/indra/lib/python3.11/site-packages/flask/app.py", line 2529, in wsgi_app
    response = self.full_dispatch_request()
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/cthoyt/.virtualenvs/indra/lib/python3.11/site-packages/flask/app.py", line 1825, in full_dispatch_request
    rv = self.handle_user_exception(e)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/cthoyt/.virtualenvs/indra/lib/python3.11/site-packages/flask/app.py", line 1823, in full_dispatch_request
    rv = self.dispatch_request()
         ^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/cthoyt/.virtualenvs/indra/lib/python3.11/site-packages/flask/app.py", line 1799, in dispatch_request
    return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/cthoyt/dev/bioregistry/src/bioregistry/app/ui.py", line 353, in reference
    return render_template(
           ^^^^^^^^^^^^^^^^
  File "/Users/cthoyt/.virtualenvs/indra/lib/python3.11/site-packages/flask/templating.py", line 147, in render_template
    return _render(app, template, context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/cthoyt/.virtualenvs/indra/lib/python3.11/site-packages/flask/templating.py", line 130, in _render
    rv = template.render(context)
         ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/cthoyt/.virtualenvs/indra/lib/python3.11/site-packages/jinja2/environment.py", line 1301, in render
    self.environment.handle_exception()
  File "/Users/cthoyt/.virtualenvs/indra/lib/python3.11/site-packages/jinja2/environment.py", line 936, in handle_exception
    raise rewrite_traceback_stack(source=source)
  File "/Users/cthoyt/dev/bioregistry/src/bioregistry/app/templates/reference.html", line 2, in top-level template code
    {% import "macros.html" as utils %}
    ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/cthoyt/dev/bioregistry/src/bioregistry/app/templates/base.html", line 164, in top-level template code
    {% block main %}
  File "/Users/cthoyt/dev/bioregistry/src/bioregistry/app/templates/base.html", line 168, in block 'main'
    {% block container %}{% endblock %}
    ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/cthoyt/dev/bioregistry/src/bioregistry/app/templates/reference.html", line 26, in block 'container'
    href="{{ fastapi_url_for("get_reference", prefix=prefix, identifier=identifier) }}?format={{ key }}">
    ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/cthoyt/.virtualenvs/indra/lib/python3.11/site-packages/starlette/applications.py", line 116, in url_path_for
    return self.router.url_path_for(__name, **path_params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/cthoyt/.virtualenvs/indra/lib/python3.11/site-packages/starlette/routing.py", line 643, in url_path_for
    return route.url_path_for(__name, **path_params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/cthoyt/.virtualenvs/indra/lib/python3.11/site-packages/starlette/routing.py", line 259, in url_path_for
    path, remaining_params = replace_params(
                             ^^^^^^^^^^^^^^^
  File "/Users/cthoyt/.virtualenvs/indra/lib/python3.11/site-packages/starlette/routing.py", line 101, in replace_params
    value = convertor.to_string(value)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/cthoyt/.virtualenvs/indra/lib/python3.11/site-packages/starlette/convertors.py", line 26, in to_string
    assert "/" not in value, "May not contain path separators"
           ^^^^^^^^^^^^^^^^
AssertionError: May not contain path separators
```

Discussion:
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/biopragmatics/bioregistry/pull/1076?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biopragmatics) Report
Attention: Patch coverage is `93.33333%` with `2 lines` in your changes are missing coverage. Please review.
> Project coverage is 40.28%. Comparing base [(`a84f6ea`)](https://app.codecov.io/gh/biopragmatics/bioregistry/commit/a84f6ea62b04d0d6170a05662f2ebcdc5dc58b1f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biopragmatics) to head [(`92071bf`)](https://app.codecov.io/gh/biopragmatics/bioregistry/pull/1076?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biopragmatics).
> Report is 16 commits behind head on main.

| [Files](https://app.codecov.io/gh/biopragmatics/bioregistry/pull/1076?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biopragmatics) | Patch % | Lines |
|---|---|---|
| [src/bioregistry/app/ui.py](https://app.codecov.io/gh/biopragmatics/bioregistry/pull/1076?src=pr&el=tree&filepath=src%2Fbioregistry%2Fapp%2Fui.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biopragmatics#diff-c3JjL2Jpb3JlZ2lzdHJ5L2FwcC91aS5weQ==) | 93.33% | [2 Missing :warning: ](https://app.codecov.io/gh/biopragmatics/bioregistry/pull/1076?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biopragmatics) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #1076      +/-   ##
==========================================
- Coverage   40.28%   40.28%   -0.01%     
==========================================
  Files         150      150              
  Lines        8308     8326      +18     
  Branches     1924     1636     -288     
==========================================
+ Hits         3347     3354       +7     
- Misses       4754     4778      +24     
+ Partials      207      194      -13     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/biopragmatics/bioregistry/pull/1076?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biopragmatics).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biopragmatics).


Patch:
diff --git a/src/bioregistry/app/api.py b/src/bioregistry/app/api.py
index 9ee3b440c..1552d82b2 100644
--- a/src/bioregistry/app/api.py
+++ b/src/bioregistry/app/api.py
@@ -466,10 +466,12 @@ class IdentifierResponse(BaseModel):
 
 
 @api_router.get(
-    "/reference/{prefix}:{identifier}", response_model=IdentifierResponse, tags=["reference"]
+    "/reference/{prefix}:{identifier:path}", response_model=IdentifierResponse, tags=["reference"]
 )
 def get_reference(request: Request, prefix: str, identifier: str):
     """Look up information on the reference."""
+    # see https://fastapi.tiangolo.com/tutorial/path-params/#path-parameters-containing-paths
+    # for more understanding on how the identifier:path handling works
     manager = request.app.manager
     resource = manager.get_resource(prefix)
     if resource is None:
@@ -478,7 +480,7 @@ def get_reference(request: Request, prefix: str, identifier: str):
     if not resource.is_standardizable_identifier(identifier):
         raise HTTPException(
             404,
-            f"invalid identifier: {resource.get_curie(identifier)} for pattern {resource.get_pattern(prefix)}",
+            f"invalid identifier: {resource.get_curie(identifier)} for pattern {resource.get_pattern()}",
         )
 
     providers = manager.get_providers(resource.prefix, identifier)
diff --git a/src/bioregistry/app/ui.py b/src/bioregistry/app/ui.py
index 7f5a6fb2d..d5dbc5865 100644
--- a/src/bioregistry/app/ui.py
+++ b/src/bioregistry/app/ui.py
@@ -290,37 +290,22 @@ def context(identifier: str):
     )
 
 
-@ui_blueprint.route("/reference/<prefix>:<path:identifier>")
-def reference(prefix: str, identifier: str):
-    """Serve a reference page."""
-    return render_template(
-        "reference.html",
-        prefix=prefix,
-        name=manager.get_name(prefix),
-        identifier=identifier,
-        providers=_get_resource_providers(prefix, identifier),
-        formats=FORMATS,
-    )
-
-
-#: this is a hack to make it work when the luid starts with a slash for
-#: ARK, since ARK doesn't actually require a slash. Will break
-#: if there are other LUIDs that actualyl require a slash in front
-ark_hacked_route = ui_blueprint.route("/<prefix>:/<path:identifier>")
+class ResponseWrapper(ValueError):
+    """An exception that helps with code reuse that returns multiple value types."""
 
+    def __init__(self, response, code=None):
+        """Instantiate this "exception", which is a tricky way of writing a macro."""
+        self.response = response
+        self.code = code
 
-@ui_blueprint.route("/<prefix>")
-@ui_blueprint.route("/<prefix>:<path:identifier>")
-@ark_hacked_route
-def resolve(prefix: str, identifier: Optional[str] = None):
-    """Resolve a CURIE.
+    def get_value(self):
+        """Get either the response, or a pair of response + code if a code is available."""
+        if self.code is not None:
+            return self.response, self.code
+        return self.response
 
-    The following things can make a CURIE unable to resolve:
 
-    1. The prefix is not registered
-    2. The prefix has a validation pattern and the identifier does not match it
-    3. There are no providers available for the URL
-    """  # noqa:DAR101,DAR201
+def _clean_reference(prefix: str, identifier: Optional[str] = None):
     if ":" in prefix:
         # A colon might appear in the prefix if there are multiple colons
         # in the CURIE, since Flask/Werkzeug parses from right to left.
@@ -333,20 +318,19 @@ def resolve(prefix: str, identifier: Optional[str] = None):
 
     _resource = manager.get_resource(prefix)
     if _resource is None:
-        return (
+        raise ResponseWrapper(
             render_template(
                 "resolve_errors/missing_prefix.html", prefix=prefix, identifier=identifier
             ),
             404,
         )
     if identifier is None:
-        return redirect(url_for("." + resource.__name__, prefix=_resource.prefix))
+        raise ResponseWrapper(redirect(url_for("." + resource.__name__, prefix=_resource.prefix)))
 
     identifier = _resource.standardize_identifier(identifier)
-
     pattern = _resource.get_pattern()
     if pattern and not _resource.is_valid_identifier(identifier):
-        return (
+        raise ResponseWrapper(
             render_template(
                 "resolve_errors/invalid_identifier.html",
                 prefix=prefix,
@@ -356,8 +340,51 @@ def resolve(prefix: str, identifier: Optional[str] = None):
             404,
         )
 
+    return _resource, identifier
+
+
+@ui_blueprint.route("/reference/<prefix>:<path:identifier>")
+@ui_blueprint.route("/reference/<prefix>:/<path:identifier>")  # ARK hack, see below
+def reference(prefix: str, identifier: str):
+    """Serve a reference page."""
+    try:
+        _resource, identifier = _clean_reference(prefix, identifier)
+    except ResponseWrapper as rw:
+        return rw.get_value()
+    return render_template(
+        "reference.html",
+        prefix=_resource.prefix,
+        name=_resource.get_name(),
+        identifier=identifier,
+        providers=_get_resource_providers(_resource.prefix, identifier),
+        formats=FORMATS,
+    )
+
+
+#: this is a hack to make it work when the luid starts with a slash for
+#: ARK, since ARK doesn't actually require a slash. Will break
+#: if there are other LUIDs that actualyl require a slash in front
+ark_hacked_route = ui_blueprint.route("/<prefix>:/<path:identifier>")
+
+
+@ui_blueprint.route("/<prefix>")
+@ui_blueprint.route("/<prefix>:<path:identifier>")
+@ark_hacked_route
+def resolve(prefix: str, identifier: Optional[str] = None):
+    """Resolve a CURIE.
+
+    The following things can make a CURIE unable to resolve:
+
+    1. The prefix is not registered
+    2. The prefix has a validation pattern and the identifier does not match it
+    3. There are no providers available for the URL
+    """  # noqa:DAR101,DAR201
+    try:
+        _resource, identifier = _clean_reference(prefix, identifier)
+    except ResponseWrapper as rw:
+        return rw.get_value()
     url = manager.get_iri(
-        prefix,
+        _resource.prefix,
         identifier,
         use_bioregistry_io=False,
         provider=request.args.get("provider"),
@@ -365,7 +392,9 @@ def resolve(prefix: str, identifier: Optional[str] = None):
     if not url:
         return (
             render_template(
-                "resolve_errors/missing_providers.html", prefix=prefix, identifier=identifier
+                "resolve_errors/missing_providers.html",
+                prefix=_resource.prefix,
+                identifier=identifier,
             ),
             404,
         )
@@ -375,7 +404,9 @@ def resolve(prefix: str, identifier: Optional[str] = None):
     except ValueError:  # headers could not be constructed
         return (
             render_template(
-                "resolve_errors/disallowed_identifier.html", prefix=prefix, identifier=identifier
+                "resolve_errors/disallowed_identifier.html",
+                prefix=_resource.prefix,
+                identifier=identifier,
             ),
             404,
         )
diff --git a/src/bioregistry/data/bioregistry.json b/src/bioregistry/data/bioregistry.json
index 26e8ddbb5..e186a3a69 100644
--- a/src/bioregistry/data/bioregistry.json
+++ b/src/bioregistry/data/bioregistry.json
@@ -22270,7 +22270,8 @@
     "pattern": "^CTRI/\\d{4}/\\d{2}/\\d+$",
     "references": [
       "https://www.ctri.nic.in/Clinicaltrials/pmaindet2.php?trialid=82487"
-    ]
+    ],
+    "uri_format": "https://trialsearch.who.int/Trial2.aspx?TrialID=$1"
   },
   "cubedb": {
     "biocontext": {


Test patch:
diff --git a/tests/test_web/test_api.py b/tests/test_web/test_api.py
index d6bd3c414..5d5a72818 100644
--- a/tests/test_web/test_api.py
+++ b/tests/test_web/test_api.py
@@ -151,10 +151,14 @@ class TestWeb(unittest.TestCase):
 
     def test_api_reference(self):
         """Test the reference endpoint."""
-        self.assert_endpoint(
+        for value in [
             "/api/reference/chebi:24867",
-            ["json", "yaml"],
-        )
+            "/api/reference/ctri:CTRI/2023/04/052053",  # check paths are accepted
+        ]:
+            self.assert_endpoint(
+                value,
+                ["json", "yaml"],
+            )
 
     def test_api_collections(self):
         """Test the collections endpoint."""
diff --git a/tests/test_web/test_ui.py b/tests/test_web/test_ui.py
index fcaebfc45..c0f1ff44a 100644
--- a/tests/test_web/test_ui.py
+++ b/tests/test_web/test_ui.py
@@ -194,3 +194,24 @@ class TestUI(unittest.TestCase):
                 with self.subTest(endpoint=endpoint):
                     res = client.get(endpoint, follow_redirects=False)
                     self.assertEqual(302, res.status_code)  # , msg=res.text)
+
+    def test_redirect_404(self):
+        """Test 404 errors."""
+        with self.app.test_client() as client:
+            for endpoint in [
+                "/chebi:abcd",  # wrong identifier pattern
+                "/gmelin:1234",  # no providers
+            ]:
+                with self.subTest(endpoint=endpoint):
+                    res = client.get(endpoint, follow_redirects=False)
+                    self.assertEqual(404, res.status_code)
+
+    def test_reference_page(self):
+        """Test the reference page."""
+        with self.app.test_client() as client:
+            for endpoint in [
+                "/reference/ctri:CTRI/2023/04/052053",  # check that slashes are okay
+            ]:
+                with self.subTest(endpoint=endpoint):
+                    res = client.get(endpoint, follow_redirects=False)
+                    self.assertEqual(200, res.status_code)