From d4e3d0686d784afd59846355136713f7abd98665 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Fri, 12 Feb 2021 16:16:51 -0500 Subject: [PATCH 1/5] RF: do not cast dataset id to an int and back to %06d I think there is no need to be "that smart" and use identifier as given. That could avoid discrepancies, such as the one I managed to inflict in https://github.com/dandi/dandi-cli/issues/302 . Thanks to @jwodder for pin pointing this in https://github.com/dandi/dandi-cli/issues/302#issuecomment-778445808 --- serve.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/serve.py b/serve.py index 72161a9..53e8057 100644 --- a/serve.py +++ b/serve.py @@ -127,30 +127,30 @@ async def goto_public_dashboard(request): return response.redirect(f"{GUI_URL}/#/dandiset") -@app.route("/dandiset/", methods=["GET", "HEAD"]) +@app.route("/dandiset/", 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 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//", methods=["GET", "HEAD"]) +@app.route("/dandiset//", 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}") + 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"]) From ad412aba28333a6c1d87fcbb68ce14e0248a26d2 Mon Sep 17 00:00:00 2001 From: "John T. Wodder II" Date: Thu, 11 Mar 2021 09:50:25 -0500 Subject: [PATCH 2/5] Fix a bug --- serve.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/serve.py b/serve.py index 53e8057..0417e22 100644 --- a/serve.py +++ b/serve.py @@ -131,7 +131,7 @@ async def goto_public_dashboard(request): async def goto_dandiset(request, dataset): """Redirect to GUI with dandiset identifier """ - req = requests.get(f"{GIRDER_LOCAL_URL}/api/v1/dandi/{dataset:06d}") + req = requests.get(f"{GIRDER_LOCAL_URL}/api/v1/dandi/{dataset}") if req.reason == "OK": url = f"{GUI_URL}/#/dandiset/{dataset}/draft" if request.method == "HEAD": From 23674871e589c5f110c2f59bf815d6983b2c4994 Mon Sep 17 00:00:00 2001 From: "John T. Wodder II" Date: Thu, 11 Mar 2021 10:07:52 -0500 Subject: [PATCH 3/5] Disable following redirects in for HEAD in tests --- test_serve.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_serve.py b/test_serve.py index d61e6c1..6bdcdc4 100644 --- a/test_serve.py +++ b/test_serve.py @@ -40,7 +40,7 @@ def test_redirect(req_url, resp_url): ], ) 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 From 7be4b520fc8e9d3fe88538285ed2faf6b651b8ee Mon Sep 17 00:00:00 2001 From: "John T. Wodder II" Date: Thu, 11 Mar 2021 10:16:05 -0500 Subject: [PATCH 4/5] Check Dandiset IDs against regex --- serve.py | 9 ++++++++- test_serve.py | 25 +++++++++++++++++-------- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/serve.py b/serve.py index 0417e22..fe3d212 100644 --- a/serve.py +++ b/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() @@ -131,6 +134,8 @@ async def goto_public_dashboard(request): async def goto_dandiset(request, dataset): """Redirect to GUI with dandiset identifier """ + 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}/draft" @@ -144,6 +149,8 @@ async def goto_dandiset(request, dataset): async def goto_dandiset_version(request, dataset, version): """Redirect to GUI with dandiset identifier and version """ + 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}/{version}" @@ -167,7 +174,7 @@ async def server_info(request): "jupyterhub": {"url": JUPYTERHUB_URL}, }, }, - indent=4 + indent=4, ) diff --git a/test_serve.py b/test_serve.py index 6bdcdc4..b892bbc 100644 --- a/test_serve.py +++ b/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,10 +26,7 @@ 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", @@ -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() From b3e0ccfacc01d8d469cc2a3da4fda8f107fe812f Mon Sep 17 00:00:00 2001 From: "John T. Wodder II" Date: Tue, 13 Apr 2021 14:28:31 -0400 Subject: [PATCH 5/5] Restrict sanic version --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 367fd93..41dffe9 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,3 @@ requests -sanic +sanic>=20.12,<20.13 sanic-cors