Merge pull request #39 from dandi/yarikoptic-patch-2

RF: do not cast dataset id to an int and back  to %06d

requirements.txt

@@ -1,3 +1,3 @@
 requests
-sanic
+sanic>=20.12,<20.13
 sanic-cors

serve.py

@@ -1,4 +1,5 @@
 import asyncio
+import re
 import os
 from sanic import Sanic
 from sanic.log import logger
@@ -22,6 +23,8 @@
     "JUPYTERHUB_URL", "https://hub.dandiarchive.org"
 ).rstrip()
 
+dandiset_identifier_regex = "^[0-9]{6}$"
+
 production = "DEV628cc89a6444" not in os.environ
 sem = None
 basedir = os.environ["HOME"] if production else os.getcwd()
@@ -127,30 +130,34 @@ async def goto_public_dashboard(request):
     return response.redirect(f"{GUI_URL}/#/dandiset")
 
 
-@app.route("/dandiset/<dataset:int>", methods=["GET", "HEAD"])
+@app.route("/dandiset/<dataset>", methods=["GET", "HEAD"])
 async def goto_dandiset(request, dataset):
     """Redirect to GUI with dandiset identifier
     """
-    req = requests.get(f"{GIRDER_LOCAL_URL}/api/v1/dandi/{dataset:06d}")
+    if not re.fullmatch(dandiset_identifier_regex, dataset):
+        return response.text(f"{dataset}: invalid Dandiset ID", status=400)
+    req = requests.get(f"{GIRDER_LOCAL_URL}/api/v1/dandi/{dataset}")
     if req.reason == "OK":
-        url = f"{GUI_URL}/#/dandiset/{dataset:06d}/draft"
+        url = f"{GUI_URL}/#/dandiset/{dataset}/draft"
         if request.method == "HEAD":
             return response.html(None, status=302, headers=make_header(url))
         return response.redirect(url)
-    return response.text(f"dandi:{dataset:06d} not found.", status=404)
+    return response.text(f"dandi:{dataset} not found.", status=404)
 
 
-@app.route("/dandiset/<dataset:int>/<version>", methods=["GET", "HEAD"])
+@app.route("/dandiset/<dataset>/<version>", methods=["GET", "HEAD"])
 async def goto_dandiset_version(request, dataset, version):
     """Redirect to GUI with dandiset identifier and version
     """
-    req = requests.get(f"{GIRDER_LOCAL_URL}/api/v1/dandi/{dataset:06d}")
+    if not re.fullmatch(dandiset_identifier_regex, dataset):
+        return response.text(f"{dataset}: invalid Dandiset ID", status=400)
+    req = requests.get(f"{GIRDER_LOCAL_URL}/api/v1/dandi/{dataset}")
     if req.reason == "OK":
-        url = f"{GUI_URL}/#/dandiset/{dataset:06d}/{version}"
+        url = f"{GUI_URL}/#/dandiset/{dataset}/{version}"
         if request.method == "HEAD":
             return response.html(None, status=302, headers=make_header(url))
         return response.redirect(url)
-    return response.text(f"dandi:{dataset:06d} not found.", status=404)
+    return response.text(f"dandi:{dataset} not found.", status=404)
 
 
 @app.route("/server-info", methods=["GET"])
@@ -167,7 +174,7 @@ async def server_info(request):
                 "jupyterhub": {"url": JUPYTERHUB_URL},
             },
         },
-        indent=4
+        indent=4,
     )
 
 

test_serve.py

@@ -10,10 +10,7 @@
         ("/", "https://gui.dandiarchive.org/"),
         ("/about", "https://www.dandiarchive.org"),
         ("/dandiset", "https://gui.dandiarchive.org/#/dandiset"),
-        (
-            "/dandiset/000003",
-            "https://gui.dandiarchive.org/#/dandiset/000003/draft",
-        ),
+        ("/dandiset/000003", "https://gui.dandiarchive.org/#/dandiset/000003/draft"),
         (
             "/dandiset/000003/0.20200703.1040",
             "https://gui.dandiarchive.org/#/dandiset/000003/0.20200703.1040",
@@ -29,18 +26,15 @@ def test_redirect(req_url, resp_url):
 @pytest.mark.parametrize(
     "req_url,resp_url",
     [
-        (
-            "/dandiset/000003",
-            "https://gui.dandiarchive.org/#/dandiset/000003/draft",
-        ),
+        ("/dandiset/000003", "https://gui.dandiarchive.org/#/dandiset/000003/draft"),
         (
             "/dandiset/000003/0.20200703.1040",
             "https://gui.dandiarchive.org/#/dandiset/000003/0.20200703.1040",
         ),
     ],
 )
 def test_redirect_head(req_url, resp_url):
-    _, r = app.test_client.head(req_url)
+    _, r = app.test_client.head(req_url, allow_redirects=False)
     r.raise_for_status()
     assert r.headers["Location"] == resp_url
     assert r.status_code == 302
@@ -59,6 +53,21 @@ def test_redirect_nonexistent_dandiset_version():
     assert r.text == f"dandi:{NEXIST_DANDI_ID} not found."
 
 
+@pytest.mark.parametrize(
+    "path,dataset",
+    [
+        ("/dandiset/12345", "12345"),
+        ("/dandiset/12345/draft", "12345"),
+        ("/dandiset/1234567", "1234567"),
+        ("/dandiset/dandiid", "dandiid"),
+    ],
+)
+def test_redirect_bad_dandiset_id(path, dataset):
+    _, r = app.test_client.get(path)
+    assert r.status_code == 400
+    assert r.text == f"{dataset}: invalid Dandiset ID"
+
+
 def test_server_info():
     _, r = app.test_client.get("/server-info")
     r.raise_for_status()