From 394979e4fceca79e98b22ff045f4a218db7c7fba Mon Sep 17 00:00:00 2001 From: omigamedev Date: Thu, 4 Jun 2026 14:56:29 +0200 Subject: [PATCH] Extract PPBR package path validation --- CMakeLists.txt | 2 + docs/modernization/build-inventory.md | 9 + docs/modernization/debt.md | 3 +- docs/modernization/roadmap.md | 14 ++ src/assets/brush_package.cpp | 129 +++++++++++++++ src/assets/brush_package.h | 53 ++++++ src/node_panel_brush.cpp | 46 +++--- tests/CMakeLists.txt | 23 ++- tests/assets/brush_package_tests.cpp | 155 ++++++++++++++++++ ...li_plan_brush_package_export_failure.cmake | 23 ++- tools/pano_cli/main.cpp | 19 +++ 11 files changed, 444 insertions(+), 32 deletions(-) create mode 100644 src/assets/brush_package.cpp create mode 100644 src/assets/brush_package.h create mode 100644 tests/assets/brush_package_tests.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index fb48841..464a204 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -97,6 +97,7 @@ target_link_libraries(pp_foundation pp_project_warnings) add_library(pp_assets STATIC + src/assets/brush_package.cpp src/assets/image_format.cpp src/assets/image_metadata.cpp src/assets/image_pixels.cpp @@ -503,6 +504,7 @@ if(PP_BUILD_APP) pp_legacy_app pp_project_options PRIVATE + pp_assets pp_project_warnings) target_precompile_headers(pp_panopainter_ui REUSE_FROM pp_legacy_app) set_target_properties(pp_panopainter_ui PROPERTIES diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index 605d02d..54d13c1 100644 --- a/docs/modernization/build-inventory.md +++ b/docs/modernization/build-inventory.md @@ -654,6 +654,15 @@ Known local toolchain state: the retained legacy `Image` header object, desktop worker-thread export, mobile/Web save completion, dialog lifetime, and success messages while brush asset/storage/UI/platform ownership is tracked by `DEBT-0047`. +- `src/assets/brush_package.*` owns the first headless PPBR package helpers: + header validation, legacy-compatible version acceptance, export path + normalization, and preview-data-directory planning. Live + `NodePanelBrushPreset::export_ppbr`/`import_ppbr` consumes these helpers, but + legacy Serializer/Image payload parsing, preview rendering, preset storage, + and strict-version cleanup remain tracked by `DEBT-0047` and `DEBT-0049`. +- `pp_assets_brush_package_tests` covers PPBR header parsing, truncated/bad + magic rejection, legacy version tolerance, export package/data path planning, + legacy extension containment, and paths the legacy regex could not match. - `pp_app_core_brush_package_export_tests` covers PPBR export request path validation, metadata preservation, legacy-flexible destination/export-data combinations, service dispatch, and malformed request rejection without diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 6c1345b..3cea81b 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -64,8 +64,9 @@ agent or engineer to remove them without reconstructing context from chat. | DEBT-0044 | Open | Modernization | Timelapse and animation MP4 export execution dispatch now consumes pure `pp_app_core` through `App::dialog_timelapse_export`, `App::dialog_export_mp4`, `pano_cli plan-export-menu`, `pano_cli plan-export-target --kind name`, `DocumentVideoExportServices`, and `src/legacy_document_export_services.*`, but the bridge still launches legacy desktop timelapse worker threads, calls `App::rec_export`, calls `Canvas::export_anim_mp4`, owns mobile/Web save callbacks, and emits success messages directly | Preserve current MP4/timelapse export behavior while video export moves toward app/document/renderer/video/platform/storage services | `pp_app_core_document_export_tests`; `pano_cli plan-export-menu --kind animation-mp4`; `pano_cli plan-export-menu --kind timelapse`; `pano_cli plan-export-target --kind name --doc-name demo --suffix -animation`; `pano_cli plan-export-target --kind name --doc-name demo --suffix -timelapse`; `ctest --preset desktop-fast --build-config Debug` | Timelapse and animation MP4 execution, desktop worker threading, frame readback/video encoding handoff, mobile/Web save callbacks, and success reporting are owned by injected app/document/renderer/video/platform/storage services with export dialogs acting only as UI adapters | | DEBT-0045 | Open | Modernization | Options-menu preference execution now consumes pure `pp_app_core` through UI scale, viewport scale, RTL direction, VR mode, VR-controller, auto-timelapse, and canvas cursor-mode callbacks plus `AppPreferenceServices` and `src/legacy_app_preference_services.*`, but the bridge still calls legacy `App::set_ui_scale`, `App::set_ui_rtl`, `App::vr_start`, `App::vr_stop`, `NodeCanvas::set_density`, `NodeCanvas::set_cursor_visibility`, `App::rec_start`, `App::rec_stop`, and `Settings::save` directly | Preserve current options-menu behavior while preferences move toward app/UI/platform/storage services | `pp_app_core_app_preferences_tests`; `pano_cli plan-app-preferences --ui-scale 1.5 --display-density 2 --current-scale 1.6 --scale-option 1 --scale-option 1.5 --rtl`; `ctest --preset desktop-fast --build-config Debug`; `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter` | Preference persistence, UI/layout direction, viewport density, cursor mode, VR mode start/stop/failure handling, VR-controller state, and auto-timelapse recording side effects are owned by injected app/UI/platform/storage services with options-menu callbacks acting only as UI adapters | | DEBT-0046 | Open | Modernization | Startup preference/runtime execution now consumes pure `pp_app_core` through `App::init`, `pano_cli plan-app-startup`, `AppStartupServices`, and `src/legacy_app_startup_services.*`, but the bridge still calls legacy `Settings::set`, `Settings::save`, `App::rec_start`, app VR-controller state mutation, and message-box license warning execution directly | Preserve current startup behavior while app startup moves toward app/preferences/storage/recording/UI services | `pp_app_core_app_startup_tests`; `pano_cli plan-app-startup --run-counter 7 --vr-controllers-disabled --license-invalid`; `pano_cli plan-app-startup --run-counter -1`; `ctest --preset desktop-fast --build-config Debug`; `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter` | Startup preference persistence, auto-timelapse startup, stored VR-controller state, license validation/warning, and startup UI/runtime side effects are owned by injected app/preferences/storage/recording/UI services with `App::init` acting only as orchestration | -| DEBT-0047 | Open | Modernization | PPBR brush package export request validation and execution dispatch now consume pure `pp_app_core` through `App::dialog_ppbr_export`, `pano_cli plan-brush-package-export`, `BrushPackageExportServices`, and `src/legacy_brush_package_export_services.*`, but the bridge still reads `NodeDialogExportPPBR`, carries the legacy `Image` header object outside the pure request, converts to `NodePanelBrushPreset::PPBRInfo`, calls `NodePanelBrushPreset::export_ppbr`, owns desktop worker-thread dispatch, dialog destruction, mobile/Web completion, and success-message behavior directly | Preserve current PPBR export behavior while brush assets, PPBR serialization, picker completion, and UI lifetime move toward asset/storage/UI/platform services | `pp_app_core_brush_package_export_tests`; `pano_cli plan-brush-package-export --path D:/Paint/clouds.ppbr --author Artist --dest-path D:/Paint/BrushPreviews --export-data --header-image`; `pano_cli plan-brush-package-export`; `pano_cli plan-brush-package-export --path D:/Paint/clouds.ppbr --dest-path D:/Paint/BrushPreviews --no-export-data`; `ctest --preset desktop-fast --build-config Debug`; `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter` | PPBR metadata collection, header-image ownership, serialization, picker-selected path execution, desktop threading, dialog lifetime, and success UI are owned by injected brush asset/storage/UI/platform services with `App::dialog_ppbr_export` acting only as a UI adapter | +| DEBT-0047 | Open | Modernization | PPBR brush package export request validation and execution dispatch now consume pure `pp_app_core` through `App::dialog_ppbr_export`, `pano_cli plan-brush-package-export`, `BrushPackageExportServices`, and `src/legacy_brush_package_export_services.*`; PPBR header/path planning now consumes `pp_assets::brush_package`, but the bridge still reads `NodeDialogExportPPBR`, carries the legacy `Image` header object outside the pure request, converts to `NodePanelBrushPreset::PPBRInfo`, calls `NodePanelBrushPreset::export_ppbr`, owns desktop worker-thread dispatch, dialog destruction, mobile/Web completion, and success-message behavior directly | Preserve current PPBR export behavior while brush assets, PPBR serialization, picker completion, and UI lifetime move toward asset/storage/UI/platform services | `pp_assets_brush_package_tests`; `pp_app_core_brush_package_export_tests`; `pano_cli plan-brush-package-export --path D:/Paint/clouds.ppbr --author Artist --dest-path D:/Paint/BrushPreviews --export-data --header-image`; `pano_cli plan-brush-package-export`; `pano_cli plan-brush-package-export --path clouds`; `pano_cli plan-brush-package-export --path D:/Paint/clouds.ppbr --dest-path D:/Paint/BrushPreviews --no-export-data`; `ctest --preset desktop-fast --build-config Debug`; `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter` | PPBR metadata collection, header-image ownership, serialization, picker-selected path execution, desktop threading, dialog lifetime, and success UI are owned by injected brush asset/storage/UI/platform services with `App::dialog_ppbr_export` acting only as a UI adapter | | DEBT-0048 | Open | Modernization | ABR/PPBR brush package import execution now consumes pure `pp_app_core` through document-open confirmation callbacks, `pano_cli plan-brush-package-import`, `BrushPackageImportServices`, and `src/legacy_brush_package_import_services.*`, but the bridge still launches detached legacy `NodePanelBrushPreset::import_abr`/`import_ppbr` worker threads and depends on the legacy preset panel as the importer/storage owner | Preserve current brush import behavior while brush package parsing, preset storage, progress/error reporting, and UI refresh move toward asset/paint/UI services | `pp_app_core_brush_package_import_tests`; `pano_cli plan-brush-package-import --kind ppbr --path D:/Paint/Brushes/clouds.ppbr`; `pano_cli plan-brush-package-import --kind abr --path D:/Paint/Brushes/clouds.abr`; `pano_cli plan-brush-package-import --kind ppbr`; `ctest --preset desktop-fast --build-config Debug`; `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter` | ABR/PPBR parsing, preset creation/storage, import threading/progress, duplicate asset policy, and UI refresh are owned by injected brush asset/paint/UI services with document-open callbacks only confirming user intent | +| DEBT-0049 | Open | Modernization | `pp_assets::validate_ppbr_header` intentionally preserves the legacy PPBR version check from `NodePanelBrushPreset::import_ppbr`, which accepts files when either major is `0` or minor is `1` instead of requiring exactly version `0.1` | Avoid rejecting existing brush packages before compatibility fixtures prove the stricter rule is safe | `pp_assets_brush_package_tests`; `pano_cli plan-brush-package-export --path D:/Paint/clouds.ppbr`; `ctest --preset desktop-fast --build-config Debug`; `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter` | Add PPBR compatibility fixtures for accepted/rejected historical package versions, then require canonical `0.1` or an explicit supported-version matrix and update live import accordingly | ## Closed Debt diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 0969980..f8da1d2 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -788,6 +788,13 @@ through the app-core brush package export executor and collection, legacy `Image` header ownership, desktop worker-thread export, mobile/Web save completion, `NodePanelBrushPreset::export_ppbr`, and existing success messages while retained execution remains tracked under `DEBT-0047`. +PPBR package header validation and export target/data-directory planning now +live in `pp_assets::brush_package` and are exercised by +`pp_assets_brush_package_tests` plus `pano_cli plan-brush-package-export`. +The live PPBR import/export path consumes those helpers, while legacy +Serializer/Image payload reading, stroke preview generation, preset storage, +and the historical permissive version check remain tracked under `DEBT-0047` +and `DEBT-0049`. Implementation tasks: @@ -1396,6 +1403,13 @@ Results: `pano_cli_plan_brush_package_import_abr_smoke`, `pano_cli_plan_brush_package_import_rejects_empty_path`, and `pano_cli_plan_brush_package_import_rejects_unknown_kind`. +- `PanoPainter`, `pp_assets_brush_package_tests`, + `pp_app_core_brush_package_export_tests`, and `pano_cli` built after PPBR + header validation and export path/data-directory planning moved into + `pp_assets`. +- Focused PPBR asset CTest coverage passed for `pp_assets_brush_package_tests` + and the brush package export CLI tests, including path-without-directory + rejection and legacy no-export-data data-directory planning. - `pp_app_core_document_recording_tests` passed, covering recording start/stop, clear, platform recorded-file cleanup, frame-count reset, export progress totals, and oversized progress-total clamping. diff --git a/src/assets/brush_package.cpp b/src/assets/brush_package.cpp new file mode 100644 index 0000000..6e6c619 --- /dev/null +++ b/src/assets/brush_package.cpp @@ -0,0 +1,129 @@ +#include "assets/brush_package.h" + +#include +#include + +namespace pp::assets { +namespace { + +[[nodiscard]] std::uint16_t read_u16_le(std::span bytes, std::size_t offset) noexcept +{ + const auto lo = static_cast(std::to_integer(bytes[offset])); + const auto hi = static_cast(std::to_integer(bytes[offset + 1U])); + return static_cast(lo | static_cast(hi << 8U)); +} + +[[nodiscard]] bool is_word_extension(std::string_view value) noexcept +{ + if (value.empty()) { + return false; + } + + for (const unsigned char ch : value) { + if (std::isalnum(ch) == 0 && ch != '_') { + return false; + } + } + + return true; +} + +} // namespace + +pp::foundation::Status validate_ppbr_header( + std::string_view magic, + std::uint16_t major, + std::uint16_t minor) noexcept +{ + if (magic != "PPBR") { + return pp::foundation::Status::invalid_argument("PPBR header magic is invalid"); + } + + // DEBT-0049: preserve legacy version acceptance until PPBR compatibility fixtures exist. + if (major != ppbr_legacy_major_version && minor != ppbr_legacy_minor_version) { + return pp::foundation::Status::invalid_argument("PPBR version is unsupported"); + } + + return pp::foundation::Status::success(); +} + +pp::foundation::Result parse_ppbr_header(std::span bytes) noexcept +{ + if (bytes.size() < ppbr_header_size) { + return pp::foundation::Result::failure( + pp::foundation::Status::out_of_range("PPBR header is truncated")); + } + + const std::string_view magic(reinterpret_cast(bytes.data()), 4U); + const auto major = read_u16_le(bytes, 4U); + const auto minor = read_u16_le(bytes, 6U); + const auto status = validate_ppbr_header(magic, major, minor); + if (!status.ok()) { + return pp::foundation::Result::failure(status); + } + + return pp::foundation::Result::success(PpbrHeader { + .major = major, + .minor = minor, + }); +} + +pp::foundation::Result normalize_ppbr_export_path(std::string_view requested_path) +{ + if (requested_path.empty()) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("PPBR export path must not be empty")); + } + + std::string path(requested_path); + if (requested_path.find(".ppbr") == std::string_view::npos) { + path += ".ppbr"; + } + + return pp::foundation::Result::success(std::move(path)); +} + +pp::foundation::Result plan_ppbr_export_paths( + std::string_view requested_path, + std::string_view override_data_directory, + bool export_data, + PpbrDataDirectoryPolicy data_directory_policy) +{ + const auto normalized = normalize_ppbr_export_path(requested_path); + if (!normalized) { + return pp::foundation::Result::failure(normalized.status()); + } + + const auto slash = normalized.value().find_last_of("/\\"); + if (slash == std::string::npos || slash + 1U >= normalized.value().size()) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("PPBR export path must include a directory and file name")); + } + + const auto dot = normalized.value().find_last_of('.'); + if (dot == std::string::npos || dot <= slash + 1U || dot + 1U >= normalized.value().size()) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("PPBR export path must include a file extension")); + } + + PpbrExportPaths paths; + paths.package_path = normalized.value(); + paths.directory = normalized.value().substr(0, slash); + paths.stem = normalized.value().substr(slash + 1U, dot - slash - 1U); + paths.extension = normalized.value().substr(dot + 1U); + if (!is_word_extension(paths.extension)) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("PPBR export path extension contains unsupported characters")); + } + + if (data_directory_policy == PpbrDataDirectoryPolicy::override_directory) { + paths.data_directory = std::string(override_data_directory) + "/" + paths.stem + "_data"; + } else { + paths.data_directory = paths.directory + "/" + paths.stem + "_data"; + } + paths.data_directory_enabled = export_data && !paths.data_directory.empty(); + + return pp::foundation::Result::success(std::move(paths)); +} + +} // namespace pp::assets diff --git a/src/assets/brush_package.h b/src/assets/brush_package.h new file mode 100644 index 0000000..e94bf76 --- /dev/null +++ b/src/assets/brush_package.h @@ -0,0 +1,53 @@ +#pragma once + +#include "foundation/result.h" + +#include +#include +#include +#include +#include + +namespace pp::assets { + +constexpr std::size_t ppbr_header_size = 8; +constexpr std::uint16_t ppbr_legacy_major_version = 0; +constexpr std::uint16_t ppbr_legacy_minor_version = 1; + +enum class PpbrDataDirectoryPolicy { + next_to_package, + override_directory, +}; + +struct PpbrHeader { + std::uint16_t major = 0; + std::uint16_t minor = 0; +}; + +struct PpbrExportPaths { + std::string package_path; + std::string directory; + std::string stem; + std::string extension; + std::string data_directory; + bool data_directory_enabled = false; +}; + +[[nodiscard]] pp::foundation::Status validate_ppbr_header( + std::string_view magic, + std::uint16_t major, + std::uint16_t minor) noexcept; + +[[nodiscard]] pp::foundation::Result parse_ppbr_header( + std::span bytes) noexcept; + +[[nodiscard]] pp::foundation::Result normalize_ppbr_export_path( + std::string_view requested_path); + +[[nodiscard]] pp::foundation::Result plan_ppbr_export_paths( + std::string_view requested_path, + std::string_view override_data_directory, + bool export_data, + PpbrDataDirectoryPolicy data_directory_policy); + +} // namespace pp::assets diff --git a/src/node_panel_brush.cpp b/src/node_panel_brush.cpp index 7b24904..735057a 100644 --- a/src/node_panel_brush.cpp +++ b/src/node_panel_brush.cpp @@ -1,6 +1,7 @@ #include "pch.h" #include "log.h" #include "node_panel_brush.h" +#include "assets/brush_package.h" #include "app_core/brush_ui.h" #include "legacy_brush_ui_services.h" #include "asset.h" @@ -679,26 +680,27 @@ void NodePanelBrushPreset::add_brush(std::shared_ptr brush) bool NodePanelBrushPreset::export_ppbr(const std::string& path_in, const PPBRInfo& info_data) { - std::string path = path_in; - if (path_in.find(".ppbr") == std::string::npos) - path += ".ppbr"; + const auto export_paths = pp::assets::plan_ppbr_export_paths( + path_in, + info_data.dest_path, + info_data.export_data, +#if __OSX__ + pp::assets::PpbrDataDirectoryPolicy::override_directory +#else + pp::assets::PpbrDataDirectoryPolicy::next_to_package +#endif + ); + if (!export_paths) { + LOG("export_ppbr invalid path: %s", export_paths.status().message); + return false; + } + + const auto& path = export_paths.value().package_path; LOG("export ppbr to: %s", path.c_str()); - std::regex r(R"((.*)[\\/]([^\\/]+)\.(\w+)?$)"); - std::smatch m; - if (!std::regex_search(path, m, r)) - return false; - auto base = m[1].str(); - auto name = m[2].str(); - auto ext = m[3].str(); + const auto& out_path = export_paths.value().data_directory; -#if __OSX__ - std::string out_path = info_data.dest_path + "/" + name + "_data"; -#else - std::string out_path = base + "/" + name + "_data"; -#endif - - bool path_created = info_data.export_data && !out_path.empty() ? Asset::create_dir(out_path) : false; + bool path_created = export_paths.value().data_directory_enabled ? Asset::create_dir(out_path) : false; std::ofstream f(path, std::ios::binary); if (f.good()) @@ -826,16 +828,12 @@ bool NodePanelBrushPreset::import_ppbr(const std::string& path) // sanity checks auto magic = sr.rstring(4); - if (magic != "PPBR") - { - LOG("PPBR tag not found") - return false; - } auto vmaj = sr.ru16(); auto vmin = sr.ru16(); - if (vmaj != 0 && vmin != 1) + const auto header_status = pp::assets::validate_ppbr_header(magic, vmaj, vmin); + if (!header_status.ok()) { - LOG("unrecognised version %d.%d", vmaj, vmin); + LOG("PPBR header rejected: %s (%d.%d)", header_status.message, vmaj, vmin); return false; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0484988..645a347 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -76,6 +76,16 @@ add_test(NAME pp_assets_image_format_tests COMMAND pp_assets_image_format_tests) set_tests_properties(pp_assets_image_format_tests PROPERTIES LABELS "assets;desktop-fast") +add_executable(pp_assets_brush_package_tests + assets/brush_package_tests.cpp) +target_link_libraries(pp_assets_brush_package_tests PRIVATE + pp_assets + pp_test_harness) + +add_test(NAME pp_assets_brush_package_tests COMMAND pp_assets_brush_package_tests) +set_tests_properties(pp_assets_brush_package_tests PROPERTIES + LABELS "assets;paint;desktop-fast;fuzz") + add_executable(pp_assets_image_metadata_tests assets/image_metadata_tests.cpp) target_link_libraries(pp_assets_image_metadata_tests PRIVATE @@ -942,7 +952,7 @@ if(TARGET pano_cli) --header-image) set_tests_properties(pano_cli_plan_brush_package_export_smoke PROPERTIES LABELS "app;paint;assets;integration;desktop-fast" - PASS_REGULAR_EXPRESSION "\"command\":\"plan-brush-package-export\".*\"path\":\"D:/Paint/clouds.ppbr\".*\"author\":\"Artist\".*\"destPath\":\"D:/Paint/BrushPreviews\".*\"exportData\":true.*\"hasHeaderImage\":true.*\"dispatches\":1") + PASS_REGULAR_EXPRESSION "\"command\":\"plan-brush-package-export\".*\"path\":\"D:/Paint/clouds.ppbr\".*\"author\":\"Artist\".*\"destPath\":\"D:/Paint/BrushPreviews\".*\"exportData\":true.*\"hasHeaderImage\":true.*\"paths\":\\{\"package\":\"D:/Paint/clouds.ppbr\".*\"dataDirectory\":\"D:/Paint/BrushPreviews/clouds_data\".*\"dataDirectoryEnabled\":true.*\"dispatches\":1") add_test(NAME pano_cli_plan_brush_package_export_rejects_empty_path COMMAND "${CMAKE_COMMAND}" @@ -952,6 +962,15 @@ if(TARGET pano_cli) set_tests_properties(pano_cli_plan_brush_package_export_rejects_empty_path PROPERTIES LABELS "app;paint;assets;integration;desktop-fast;fuzz") + add_test(NAME pano_cli_plan_brush_package_export_rejects_path_without_directory + COMMAND "${CMAKE_COMMAND}" + -DPANO_CLI=$ + -DEXPECT_NO_DIRECTORY=ON + "-DEXPECTED_OUTPUT=PPBR export path must include a directory and file name" + -P "${CMAKE_CURRENT_SOURCE_DIR}/cmake/expect_pano_cli_plan_brush_package_export_failure.cmake") + set_tests_properties(pano_cli_plan_brush_package_export_rejects_path_without_directory PROPERTIES + LABELS "app;paint;assets;integration;desktop-fast;fuzz") + add_test(NAME pano_cli_plan_brush_package_export_dest_without_data_smoke COMMAND pano_cli plan-brush-package-export --path D:/Paint/clouds.ppbr @@ -959,7 +978,7 @@ if(TARGET pano_cli) --no-export-data) set_tests_properties(pano_cli_plan_brush_package_export_dest_without_data_smoke PROPERTIES LABELS "app;paint;assets;integration;desktop-fast;fuzz" - PASS_REGULAR_EXPRESSION "\"command\":\"plan-brush-package-export\".*\"destPath\":\"D:/Paint/BrushPreviews\".*\"exportData\":false.*\"dispatches\":1") + PASS_REGULAR_EXPRESSION "\"command\":\"plan-brush-package-export\".*\"destPath\":\"D:/Paint/BrushPreviews\".*\"exportData\":false.*\"dataDirectory\":\"D:/Paint/BrushPreviews/clouds_data\".*\"dataDirectoryEnabled\":false.*\"dispatches\":1") add_test(NAME pano_cli_plan_tools_menu_shortcuts_smoke COMMAND pano_cli plan-tools-menu --command shortcuts) diff --git a/tests/assets/brush_package_tests.cpp b/tests/assets/brush_package_tests.cpp new file mode 100644 index 0000000..2366c05 --- /dev/null +++ b/tests/assets/brush_package_tests.cpp @@ -0,0 +1,155 @@ +#include "assets/brush_package.h" +#include "test_harness.h" + +#include +#include +#include +#include + +namespace { + +std::array ppbr_header(std::uint16_t major, std::uint16_t minor) +{ + return { + std::byte { 'P' }, + std::byte { 'P' }, + std::byte { 'B' }, + std::byte { 'R' }, + static_cast(major & 0xffU), + static_cast((major >> 8U) & 0xffU), + static_cast(minor & 0xffU), + static_cast((minor >> 8U) & 0xffU), + }; +} + +void parses_ppbr_header_and_legacy_version_tolerance(pp::tests::Harness& harness) +{ + const auto canonical_bytes = ppbr_header(0, 1); + const auto canonical = pp::assets::parse_ppbr_header(canonical_bytes); + + PP_EXPECT(harness, canonical); + PP_EXPECT(harness, canonical.value().major == 0U); + PP_EXPECT(harness, canonical.value().minor == 1U); + + const auto minor_tolerated_bytes = ppbr_header(0, 2); + const auto major_tolerated_bytes = ppbr_header(1, 1); + const auto rejected_bytes = ppbr_header(1, 2); + const auto legacy_minor_tolerated = pp::assets::parse_ppbr_header(minor_tolerated_bytes); + const auto legacy_major_tolerated = pp::assets::parse_ppbr_header(major_tolerated_bytes); + const auto rejected = pp::assets::parse_ppbr_header(rejected_bytes); + PP_EXPECT(harness, legacy_minor_tolerated); + PP_EXPECT(harness, legacy_major_tolerated); + PP_EXPECT(harness, !rejected); +} + +void rejects_truncated_and_bad_magic_headers(pp::tests::Harness& harness) +{ + const std::array truncated { + std::byte { 'P' }, + std::byte { 'P' }, + std::byte { 'B' }, + std::byte { 'R' }, + }; + auto bad_magic = ppbr_header(0, 1); + bad_magic[2] = std::byte { 'X' }; + + const auto truncated_result = pp::assets::parse_ppbr_header(truncated); + const auto magic_result = pp::assets::parse_ppbr_header(bad_magic); + + PP_EXPECT(harness, !truncated_result); + PP_EXPECT(harness, truncated_result.status().code == pp::foundation::StatusCode::out_of_range); + PP_EXPECT(harness, !magic_result); + PP_EXPECT(harness, magic_result.status().code == pp::foundation::StatusCode::invalid_argument); +} + +void plans_export_package_and_data_paths(pp::tests::Harness& harness) +{ + const auto regular = pp::assets::plan_ppbr_export_paths( + "D:/Paint/clouds", + "", + true, + pp::assets::PpbrDataDirectoryPolicy::next_to_package); + PP_EXPECT(harness, regular); + if (regular) { + PP_EXPECT(harness, regular.value().package_path == "D:/Paint/clouds.ppbr"); + PP_EXPECT(harness, regular.value().directory == "D:/Paint"); + PP_EXPECT(harness, regular.value().stem == "clouds"); + PP_EXPECT(harness, regular.value().extension == "ppbr"); + PP_EXPECT(harness, regular.value().data_directory == "D:/Paint/clouds_data"); + PP_EXPECT(harness, regular.value().data_directory_enabled); + } + + const auto override = pp::assets::plan_ppbr_export_paths( + "/brushes/clouds.ppbr", + "/Users/artist/Exports", + true, + pp::assets::PpbrDataDirectoryPolicy::override_directory); + PP_EXPECT(harness, override); + if (override) { + PP_EXPECT(harness, override.value().data_directory == "/Users/artist/Exports/clouds_data"); + PP_EXPECT(harness, override.value().data_directory_enabled); + } + + const auto no_data = pp::assets::plan_ppbr_export_paths( + "D:/Paint/clouds.ppbr", + "", + false, + pp::assets::PpbrDataDirectoryPolicy::next_to_package); + PP_EXPECT(harness, no_data); + if (no_data) { + PP_EXPECT(harness, !no_data.value().data_directory_enabled); + } +} + +void preserves_legacy_extension_containment_rule(pp::tests::Harness& harness) +{ + const auto path = pp::assets::normalize_ppbr_export_path("D:/Paint/clouds.ppbr.tmp"); + + PP_EXPECT(harness, path); + PP_EXPECT(harness, path.value() == "D:/Paint/clouds.ppbr.tmp"); +} + +void rejects_export_paths_that_legacy_regex_could_not_match(pp::tests::Harness& harness) +{ + PP_EXPECT( + harness, + !pp::assets::plan_ppbr_export_paths( + "", + "", + true, + pp::assets::PpbrDataDirectoryPolicy::next_to_package)); + PP_EXPECT( + harness, + !pp::assets::plan_ppbr_export_paths( + "clouds", + "", + true, + pp::assets::PpbrDataDirectoryPolicy::next_to_package)); + PP_EXPECT( + harness, + !pp::assets::plan_ppbr_export_paths( + "D:/Paint/.ppbr", + "", + true, + pp::assets::PpbrDataDirectoryPolicy::next_to_package)); + PP_EXPECT( + harness, + !pp::assets::plan_ppbr_export_paths( + "D:/Paint/clouds.ppbr!", + "", + true, + pp::assets::PpbrDataDirectoryPolicy::next_to_package)); +} + +} // namespace + +int main() +{ + pp::tests::Harness harness; + harness.run("parses PPBR header and legacy version tolerance", parses_ppbr_header_and_legacy_version_tolerance); + harness.run("rejects truncated and bad magic headers", rejects_truncated_and_bad_magic_headers); + harness.run("plans export package and data paths", plans_export_package_and_data_paths); + harness.run("preserves legacy extension containment rule", preserves_legacy_extension_containment_rule); + harness.run("rejects export paths that legacy regex could not match", rejects_export_paths_that_legacy_regex_could_not_match); + return harness.finish(); +} diff --git a/tests/cmake/expect_pano_cli_plan_brush_package_export_failure.cmake b/tests/cmake/expect_pano_cli_plan_brush_package_export_failure.cmake index 569feec..018e47e 100644 --- a/tests/cmake/expect_pano_cli_plan_brush_package_export_failure.cmake +++ b/tests/cmake/expect_pano_cli_plan_brush_package_export_failure.cmake @@ -6,11 +6,24 @@ if(NOT DEFINED EXPECTED_OUTPUT) message(FATAL_ERROR "EXPECTED_OUTPUT must be set") endif() -execute_process( - COMMAND "${PANO_CLI}" plan-brush-package-export - RESULT_VARIABLE result - OUTPUT_VARIABLE output - ERROR_VARIABLE error) +if(NOT DEFINED EXPECT_NO_DIRECTORY) + set(EXPECT_NO_DIRECTORY OFF) +endif() + +if(EXPECT_NO_DIRECTORY) + execute_process( + COMMAND "${PANO_CLI}" plan-brush-package-export + --path clouds + RESULT_VARIABLE result + OUTPUT_VARIABLE output + ERROR_VARIABLE error) +else() + execute_process( + COMMAND "${PANO_CLI}" plan-brush-package-export + RESULT_VARIABLE result + OUTPUT_VARIABLE output + ERROR_VARIABLE error) +endif() if(result EQUAL 0) message(FATAL_ERROR "Expected pano_cli plan-brush-package-export to fail, but it exited 0") diff --git a/tools/pano_cli/main.cpp b/tools/pano_cli/main.cpp index 03d8b04..47306b9 100644 --- a/tools/pano_cli/main.cpp +++ b/tools/pano_cli/main.cpp @@ -20,6 +20,7 @@ #include "app_core/document_sharing.h" #include "app_core/document_session.h" #include "app_core/file_menu.h" +#include "assets/brush_package.h" #include "app_core/grid_ui.h" #include "app_core/history_ui.h" #include "app_core/main_toolbar.h" @@ -3671,6 +3672,18 @@ int plan_brush_package_export(int argc, char** argv) return 2; } + const auto paths = pp::assets::plan_ppbr_export_paths( + args.path, + args.destination_path, + args.export_data, + args.destination_path.empty() + ? pp::assets::PpbrDataDirectoryPolicy::next_to_package + : pp::assets::PpbrDataDirectoryPolicy::override_directory); + if (!paths) { + print_error("plan-brush-package-export", paths.status().message); + return 2; + } + std::cout << "{\"ok\":true,\"command\":\"plan-brush-package-export\"" << ",\"request\":{\"path\":\"" << json_escape(services.last_path) << "\",\"author\":\"" << json_escape(services.last_request.author) @@ -3680,6 +3693,12 @@ int plan_brush_package_export(int argc, char** argv) << "\",\"destPath\":\"" << json_escape(services.last_request.destination_path) << "\",\"exportData\":" << json_bool(services.last_request.export_data) << ",\"hasHeaderImage\":" << json_bool(services.last_request.has_header_image) + << "},\"paths\":{\"package\":\"" << json_escape(paths.value().package_path) + << "\",\"directory\":\"" << json_escape(paths.value().directory) + << "\",\"stem\":\"" << json_escape(paths.value().stem) + << "\",\"extension\":\"" << json_escape(paths.value().extension) + << "\",\"dataDirectory\":\"" << json_escape(paths.value().data_directory) + << "\",\"dataDirectoryEnabled\":" << json_bool(paths.value().data_directory_enabled) << "},\"dispatches\":" << services.exports << "}\n"; return 0;