From 7fc1a59f0e5823d7c275e74291d4e1c0566b650f Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Thu, 6 Jun 2024 14:38:28 +0200 Subject: [PATCH] Manifest: transform path before checking against the zip file list --- extensions/tests/test_submit.py | 2 +- files/tests/test_utils.py | 83 +++++++++++++++++++++++++++++++++ files/utils.py | 31 ++++++++++-- 3 files changed, 110 insertions(+), 6 deletions(-) diff --git a/extensions/tests/test_submit.py b/extensions/tests/test_submit.py index 300b63da..268b3096 100644 --- a/extensions/tests/test_submit.py +++ b/extensions/tests/test_submit.py @@ -102,7 +102,7 @@ EXPECTED_VALIDATION_ERRORS = { 'invalid-theme-multiple-xmls.zip': {'source': ['A theme should have exactly one XML file.']}, 'invalid-missing-wheels.zip': { 'source': [ - 'A declared wheel is missing in the zip file, expected path: addon/./wheels/test-wheel-whatever.whl' + 'A declared wheel is missing in the zip file, expected path: addon/wheels/test-wheel-whatever.whl' ] }, } diff --git a/files/tests/test_utils.py b/files/tests/test_utils.py index 2890d9e3..20d0a2c4 100644 --- a/files/tests/test_utils.py +++ b/files/tests/test_utils.py @@ -1,5 +1,6 @@ from pathlib import Path from unittest.mock import patch, ANY +import dataclasses import tempfile from django.test import TestCase @@ -11,6 +12,7 @@ from files.utils import ( find_path_by_name, get_thumbnail_upload_to, make_thumbnails, + validate_file_list, ) # Reusing test files from the extensions app @@ -171,3 +173,84 @@ class UtilsTest(TestCase): mock_ffmpeg.return_value.option.return_value.input.assert_any_call( 'path/to/source/video.mp4' ) + + def test_validate_file_list(self): + @dataclasses.dataclass + class TestParams: + name: str + toml_content: dict + manifest_filepath: dict + file_list: list + expected: list + + for test in [ + TestParams( + name='valid add-on', + toml_content={'type': 'add-on'}, + manifest_filepath='blender_manifest.toml', + file_list=['__init__.py'], + expected=[], + ), + TestParams( + name='add-on missing init', + toml_content={'type': 'add-on'}, + manifest_filepath='blender_manifest.toml', + file_list=[], + expected=['invalid_missing_init'], + ), + TestParams( + name='valid theme', + toml_content={'type': 'theme'}, + manifest_filepath='blender_manifest.toml', + file_list=['theme.xml'], + expected=[], + ), + TestParams( + name='missing theme file', + toml_content={'type': 'theme'}, + manifest_filepath='blender_manifest.toml', + file_list=[], + expected=['missing_or_multiple_theme_xml'], + ), + TestParams( + name='multiple theme files', + toml_content={'type': 'theme'}, + manifest_filepath='blender_manifest.toml', + file_list=['theme.xml', 'theme/2.xml'], + expected=['missing_or_multiple_theme_xml'], + ), + TestParams( + name='valid nested wheels', + toml_content={'type': 'add-on', 'wheels': ['wheels/1.whl']}, + manifest_filepath='addon/blender_manifest.toml', + file_list=['addon/__init__.py', 'addon/wheels/1.whl'], + expected=[], + ), + TestParams( + name='valid nested wheels with ./', + toml_content={'type': 'add-on', 'wheels': ['./wheels/1.whl']}, + manifest_filepath='blender_manifest.toml', + file_list=['__init__.py', 'wheels/1.whl'], + expected=[], + ), + TestParams( + name='missing wheel', + toml_content={'type': 'add-on', 'wheels': ['./wheels/1.whl', './wheels/2.whl']}, + manifest_filepath='blender_manifest.toml', + file_list=['__init__.py', 'wheels/1.whl'], + expected=[{'code': 'missing_wheel', 'params': {'path': 'wheels/2.whl'}}], + ), + TestParams( + name='incorrect wheel nesting', + toml_content={'type': 'add-on', 'wheels': ['./wheels/1.whl']}, + manifest_filepath='add-on/blender_manifest.toml', + file_list=['add-on/__init__.py', 'wheels/1.whl'], + expected=[{'code': 'missing_wheel', 'params': {'path': 'add-on/wheels/1.whl'}}], + ), + ]: + with self.subTest(**dataclasses.asdict(test)): + self.assertEqual( + test.expected, + validate_file_list(test.toml_content, test.manifest_filepath, test.file_list), + test.name, + ) diff --git a/files/utils.py b/files/utils.py index 41eb2d9d..061de00f 100644 --- a/files/utils.py +++ b/files/utils.py @@ -147,7 +147,14 @@ def read_manifest_from_zip(archive_path): error_codes.append('invalid_manifest_toml') return None, error_codes - # If manifest was parsed successfully, do additional type-specific validation + file_list_error_codes = validate_file_list(toml_content, manifest_filepath, file_list) + error_codes.extend(file_list_error_codes) + return toml_content, error_codes + + +def validate_file_list(toml_content, manifest_filepath, file_list): + """Check the files in in the archive against manifest.""" + error_codes = [] type_slug = toml_content['type'] if type_slug == 'theme': theme_xmls = filter_paths_by_ext(file_list, '.xml') @@ -155,22 +162,36 @@ def read_manifest_from_zip(archive_path): error_codes.append('missing_or_multiple_theme_xml') elif type_slug == 'add-on': # __init__.py is expected to be next to the manifest - expected_init_path = os.path.join(os.path.dirname(manifest_filepath), '__init__.py') + expected_init_path = _canonical_path('__init__.py', manifest_filepath) init_filepath = find_exact_path(file_list, expected_init_path) if not init_filepath: error_codes.append('invalid_missing_init') - wheels = toml_content.get('wheels') if wheels: for wheel in wheels: - expected_wheel_path = os.path.join(os.path.dirname(manifest_filepath), wheel) + expected_wheel_path = _canonical_path(wheel, manifest_filepath) wheel_filepath = find_exact_path(file_list, expected_wheel_path) if not wheel_filepath: error_codes.append( {'code': 'missing_wheel', 'params': {'path': expected_wheel_path}} ) + return error_codes - return toml_content, error_codes + +def _canonical_path(path, manifest_filepath): + """Transform path before checking against the zip file list. + + We expect to support other manifest fields (e.g. in the [build] section) that will potentially + point to directories, including the "current" directory, which has to be denoted as "./". + To avoid inconsistencies in file path notations supported for different fields, we process all + paths values in manifest in a uniform way, allowing the leading "./" in all file paths. + + All paths mentioned in manifest are treated as relative for the directory that contains + manifest_filepath. + """ + if path.startswith('./'): + path = path[2:] + return os.path.join(os.path.dirname(manifest_filepath), path) def guess_mimetype_from_ext(file_name: str) -> str: -- 2.30.2