dandi / dandi/dandi-archive

Archive handles "extra" slashes in Zarr entries poorly

Open
#1,835 0 comments 0 reactions 0 assignees View on GitHub
bug zarr
Dominant language
Python
Stars
26
Forks
21
Avg merge
4d 23h
Merged PRs (30d)
15

Description

When uploading entries to a Zarr, entry paths of the form `/`, `foo/`, `/foo`, and `foo//bar` will all be accepted, and the resulting keys on S3 will just be of the form `zarr/{zarr_id}/{path}`, using the paths as-is. However, listing the Zarr entries afterwards via the Archive's `/zarr/{zarr_id}/files/` endpoint will not return the same values:

- `/` becomes `.`
- `foo/` and `/foo` both become `foo` (even if both are present in the same Zarr)
- `foo//bar` becomes `foo/bar`

In addition, fetching a Zarr entry's download URL from the Archive via `/zarr/{zarr_id}/files/?prefix={path}&download=true` will return a URL of the form `https://{bucket}.s3.amazonaws/zarr/{zarr_id}/{mangled_path}?{params}`, where `{mangled_path}` is the path as reported by `/zarr/{zarr_id}/files/` (except in the case of `/` → `.`, where the path portion of the download URL is just `/zarr/{zarr_id}` without trailing `/.`). As `{mangled_path}` does not match the entry paths on S3, these download URLs will return 404.

I would recommend addressing this by simply forbidding creation of Zarr entries that start or end with a forward slash or contain two or more consecutive forward slashes; cf. #1111.

MVCE:

```python
from base64 import b64encode
from dataclasses import dataclass
import hashlib
import os
import boto3
from botocore import UNSIGNED
from botocore.client import Config
from dandi.dandiapi import DandiAPIClient, RESTFullAPIClient

INSTANCE = "dandi-staging"
BUCKET = "dandi-api-staging-dandisets"

FILES = [
"foo/",
"/foo",
"/",
"foo//bar",
]

@dataclass
class Entry:
path: str
blob: bytes
base64md5: str

ENTRIES = []
for path in FILES:
blob = f"path={path!r}\n".encode("utf-8")
base64md5 = b64encode(hashlib.md5(blob).digest()).decode("us-ascii")
ENTRIES.append(Entry(path, blob, base64md5))

with DandiAPIClient.for_dandi_instance(
INSTANCE, token=os.environ["DANDI_API_KEY"]
) as client:
d = client.create_dandiset(
"Test Dandiset",
{
"schemaKey": "Dandiset",
"name": "Test Dandiset",
"description": "A test Dandiset",
"contributor": [
{
"schemaKey": "Person",
"name": "Wodder, John",
"roleName": ["dcite:Author", "dcite:ContactPerson"],
}
],
"license": ["spdx:CC0-1.0"],
},
)

dandiset_id = d.identifier
print("DANDISET ID:", dandiset_id)
try:
zarr_id = client.post(
"/zarr/", json={"name": "evil.zarr", "dandiset": dandiset_id}
)["zarr_id"]
r = client.post(
f"{d.version_api_path}assets/",
json={
"metadata": {"path": "evil.zarr", "description": "An evil Zarr"},
"zarr_id": zarr_id,
},
)

uploading = [{"path": e.path, "base64md5": e.base64md5} for e in ENTRIES]
r = client.post(f"/zarr/{zarr_id}/files/", json=uploading)
with RESTFullAPIClient(
"http://nil.nil",
headers={"X-Amz-ACL": "bucket-owner-full-control"},
) as storage:
for signed_url, e in zip(r, ENTRIES):
storage.put(
signed_url,
data=e.blob,
json_resp=False,
headers={"Content-MD5": e.base64md5},
)
client.post(f"/zarr/{zarr_id}/finalize/")

asset = d.get_asset_by_path("evil.zarr")
print("Files in evil.zarr, per Archive:")
for entry in asset.iterfiles():
print(" -", repr(str(entry)))
dlurl = client.request(
"HEAD",
f"/zarr/{zarr_id}/files/",
params={"prefix": str(entry), "download": "true"},
json_resp=False,
allow_redirects=False,
).headers.get("Location")
print(f" - S3 download URL: {dlurl}")
print()
print("Files in evil.zarr, per S3:")
s3 = boto3.client("s3", config=Config(signature_version=UNSIGNED))
prefix = f"zarr/{asset.zarr}/"
for page in s3.get_paginator("list_objects_v2").paginate(
Bucket=BUCKET, Prefix=prefix
):
for obj in page["Contents"]:
path = obj["Key"].removeprefix(prefix)
print(" -", repr(path))

finally:
print("Deleting Dandiset ...")
d.delete()
```

Sample output:

```text
DANDISET ID: 212192
Files in evil.zarr, per Archive:
- '.'
- S3 download URL: https://dandi-api-staging-dandisets.s3.amazonaws.com/zarr/d81ba852-0b5e-4e3d-b34f-e946dbbfe30c?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAUBRWC5GAI6UW6OBI%2F20240126%2Fus-east-2%2Fs3%2Faws4_request&X-Amz-Date=20240126T133403Z&X-Amz-Expires=3600&X-Amz-SignedHeaders=host&X-Amz-Signature=2ddaf9198cc82e77f50215538d8a3e1eca3442f47d6be26d43491d75afd61999
- 'foo'
- S3 download URL: https://dandi-api-staging-dandisets.s3.amazonaws.com/zarr/d81ba852-0b5e-4e3d-b34f-e946dbbfe30c/foo?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAUBRWC5GAI6UW6OBI%2F20240126%2Fus-east-2%2Fs3%2Faws4_request&X-Amz-Date=20240126T133403Z&X-Amz-Expires=3600&X-Amz-SignedHeaders=host&X-Amz-Signature=9ea2421d085f45d289338165780be350116332d0177f4e34bdf7fbf651cbadec
- 'foo'
- S3 download URL: https://dandi-api-staging-dandisets.s3.amazonaws.com/zarr/d81ba852-0b5e-4e3d-b34f-e946dbbfe30c/foo?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAUBRWC5GAI6UW6OBI%2F20240126%2Fus-east-2%2Fs3%2Faws4_request&X-Amz-Date=20240126T133403Z&X-Amz-Expires=3600&X-Amz-SignedHeaders=host&X-Amz-Signature=9ea2421d085f45d289338165780be350116332d0177f4e34bdf7fbf651cbadec
- 'foo/bar'
- S3 download URL: https://dandi-api-staging-dandisets.s3.amazonaws.com/zarr/d81ba852-0b5e-4e3d-b34f-e946dbbfe30c/foo/bar?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAUBRWC5GAI6UW6OBI%2F20240126%2Fus-east-2%2Fs3%2Faws4_request&X-Amz-Date=20240126T133403Z&X-Amz-Expires=3600&X-Amz-SignedHeaders=host&X-Amz-Signature=7f7cbc17b7f393566827fcabffadff85d2b64c2031d1453460d90345a187097f

Files in evil.zarr, per S3:
- '/'
- '/foo'
- 'foo/'
- 'foo//bar'
Deleting Dandiset ...
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the POST /zarr/{zarr_id}/files/ entry point and compare its accepted paths with the listing and download behavior described in the MVCE. Use #1111 as related context, then add coverage for '/', 'foo/', '/foo', and 'foo//bar'; done means those invalid paths are rejected before upload.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.