diff --git a/dandiapi/api/tests/factories.py b/dandiapi/api/tests/factories.py index 31090804d..eace897e9 100644 --- a/dandiapi/api/tests/factories.py +++ b/dandiapi/api/tests/factories.py @@ -174,7 +174,7 @@ class DraftAssetFactory(factory.django.DjangoModelFactory): class Meta: model = Asset - path = factory.Faker('file_path', extension='nwb') + path = factory.Faker('file_path', absolute=False, extension='nwb') blob = factory.SubFactory(AssetBlobFactory) @factory.lazy_attribute diff --git a/dandiapi/api/tests/test_asset.py b/dandiapi/api/tests/test_asset.py index 680f9de53..c59cf0f74 100644 --- a/dandiapi/api/tests/test_asset.py +++ b/dandiapi/api/tests/test_asset.py @@ -538,6 +538,47 @@ def test_asset_create(api_client, user, draft_version, asset_blob): assert draft_version.status == Version.Status.PENDING +@pytest.mark.parametrize( + 'path,expected_status_code', + [ + ('foo.txt', 200), + ('/foo', 400), + ('', 400), + ('/', 400), + ('./foo', 400), + ('../foo', 400), + ('foo/.', 400), + ('foo/..', 400), + ('foo/./bar', 400), + ('foo/../bar', 400), + ('foo//bar', 400), + ('foo\0bar', 400), + ('foo/.bar', 200), + ], +) +@pytest.mark.django_db +def test_asset_create_path_validation( + api_client, user, draft_version, asset_blob, path, expected_status_code +): + assign_perm('owner', user, draft_version.dandiset) + api_client.force_authenticate(user=user) + + metadata = { + 'schemaVersion': settings.DANDI_SCHEMA_VERSION, + 'encodingFormat': 'application/x-nwb', + 'path': path, + } + + resp = api_client.post( + f'/api/dandisets/{draft_version.dandiset.identifier}' + f'/versions/{draft_version.version}/assets/', + {'metadata': metadata, 'blob_id': asset_blob.blob_id}, + format='json', + ) + + assert resp.status_code == expected_status_code, resp.data + + @pytest.mark.django_db def test_asset_create_embargo(api_client, user, draft_version, embargoed_asset_blob): assign_perm('owner', user, draft_version.dandiset) @@ -740,7 +781,7 @@ def test_asset_create_no_path(api_client, user, draft_version, asset_blob): format='json', ) assert resp.status_code == 400 - assert resp.data == {'metadata': ['No path specified in metadata.']} + assert resp.data == {'metadata': ['No path specified in metadata.']}, resp.data @pytest.mark.django_db diff --git a/dandiapi/api/views/asset.py b/dandiapi/api/views/asset.py index 8e772c274..78d7c165f 100644 --- a/dandiapi/api/views/asset.py +++ b/dandiapi/api/views/asset.py @@ -14,6 +14,7 @@ MinioStorage = type('FakeMinioStorage', (), {}) import os.path +from pathlib import PurePosixPath from urllib.parse import urlencode from django.core.paginator import EmptyPage, Page, Paginator @@ -250,10 +251,31 @@ def validate(self, data): raise serializers.ValidationError( {'blob_id': 'Exactly one of blob_id or zarr_id must be specified.'} ) - - if 'path' not in data['metadata']: + if 'path' not in data['metadata'] or not data['metadata']['path']: raise serializers.ValidationError({'metadata': 'No path specified in metadata.'}) + path = data['metadata']['path'] + # paths starting with / + if PurePosixPath(path).is_absolute(): + raise serializers.ValidationError( + {'metadata': 'Absolute path not allowed for an asset'} + ) + + sub_paths = path.split('/') + # checking for . in path + for sub_path in sub_paths: + if len(set(sub_path)) == 1 and sub_path[0] == '.': + raise serializers.ValidationError( + {'metadata': 'Invalid characters (.) in file path'} + ) + + # match characters repeating more than once + multiple_occurrence_regex = '[/]{2,}' + if '\0' in path: + raise serializers.ValidationError({'metadata': 'Invalid characters (\0) in file name'}) + if re.search(multiple_occurrence_regex, path): + raise serializers.ValidationError({'metadata': 'Invalid characters (/)) in file name'}) + return data