Commit fe93ba03f9 for woocommerce
commit fe93ba03f90238be643ef17ec6a629bdf2849125
Author: Chi-Hsuan Huang <chihsuan.tw@gmail.com>
Date: Fri Apr 25 09:32:30 2025 +0800
[Blueprint] Improve schema/filter validation (#57270)
* Enhance ExportSchema class with landing page validation and exporter filtering
- Added validation for the landing page path to ensure it is a valid relative local URL, throwing an InvalidArgumentException for invalid paths.
- Refactored the landing page assignment to use a variable for clarity.
- Implemented filtering of exporters to ensure only instances of StepExporter are included, improving the integrity of the export process.
* Enhance ImportSchema class with improved error handling and validation
- Added exception documentation for the `create_from_file` and `create_from_json` methods to clarify potential errors.
- Implemented validation for step processors to ensure they are instances of StepProcessor, improving the integrity of the import process.
- Updated JSON validation to use `wp_json_encode`, enhancing compatibility with WordPress standards.
* Validate importer type in ImportStep to ensure it is a valid StepProcessor before processing, enhancing error handling and integrity of the import process.
* Enhance ExportSchema and ImportStep tests with additional validation scenarios
- Updated ExportSchemaTest to validate landing page path and throw an exception for invalid paths.
- Added a new test in ImportStepTest to ensure warnings are returned when an importer is not a valid StepProcessor, improving test coverage and error handling.
* Refactor ExportSchema to return WP_Error for invalid landing page paths
- Updated the export method to return a WP_Error instead of throwing an exception for invalid landing page paths, improving error handling.
- Modified the corresponding test to validate the new behavior, ensuring it returns a WP_Error instance when the path is invalid.
* Improve error handling in ExportCli by checking for WP_Error in export result
- Updated the export method in ExportCli to handle WP_Error responses from ExportSchema, providing clearer error messages in the CLI.
- Ensured that the export result is properly encoded and saved only if no errors occur, enhancing the robustness of the export process.
* Add test to filter out invalid exporters in ExportSchemaTest
- Implemented a new test method to ensure that only valid instances of StepExporter are included in the export process.
- Added mock exporters for testing, including an invalid exporter that throws an exception, to validate the filtering logic.
- Enhanced test coverage for the ExportSchema class by verifying the correct handling of invalid exporters.
* Add changelog
diff --git a/packages/php/blueprint/changelog/improve-blueprint-validation b/packages/php/blueprint/changelog/improve-blueprint-validation
new file mode 100644
index 0000000000..5e75b41f75
--- /dev/null
+++ b/packages/php/blueprint/changelog/improve-blueprint-validation
@@ -0,0 +1,4 @@
+Significance: patch
+Type: enhancement
+
+Improve schema/filter validation
diff --git a/packages/php/blueprint/src/Cli/ExportCli.php b/packages/php/blueprint/src/Cli/ExportCli.php
index 2f25a503ec..82c79fb0c1 100644
--- a/packages/php/blueprint/src/Cli/ExportCli.php
+++ b/packages/php/blueprint/src/Cli/ExportCli.php
@@ -43,8 +43,13 @@ class ExportCli {
$exporter = new ExportSchema();
- $schema = $exporter->export( $args['steps'] );
- $is_saved = $this->wp_filesystem_put_contents( $this->save_to, wp_json_encode( $schema, JSON_PRETTY_PRINT ) );
+ $result = $exporter->export( $args['steps'] );
+ if ( is_wp_error( $result ) ) {
+ \WP_CLI::error( $result->get_error_message() );
+ return;
+ }
+
+ $is_saved = $this->wp_filesystem_put_contents( $this->save_to, wp_json_encode( $result, JSON_PRETTY_PRINT ) );
if ( false === $is_saved ) {
\WP_CLI::error( "Failed to save to {$this->save_to}" );
diff --git a/packages/php/blueprint/src/ExportSchema.php b/packages/php/blueprint/src/ExportSchema.php
index c241a75a29..b58f9d4bd8 100644
--- a/packages/php/blueprint/src/ExportSchema.php
+++ b/packages/php/blueprint/src/ExportSchema.php
@@ -3,9 +3,8 @@
namespace Automattic\WooCommerce\Blueprint;
use Automattic\WooCommerce\Blueprint\Exporters\StepExporter;
-use Automattic\WooCommerce\Blueprint\Exporters\ExportInstallPluginSteps;
-use Automattic\WooCommerce\Blueprint\Exporters\ExportInstallThemeSteps;
use Automattic\WooCommerce\Blueprint\Exporters\HasAlias;
+use WP_Error;
/**
* Class ExportSchema
@@ -39,11 +38,27 @@ class ExportSchema {
*
* @param string[] $steps Array of step names to export, optional.
*
- * @return array The exported schema array.
+ * @return array|WP_Error The exported schema array or a WP_Error if the export fails.
*/
public function export( $steps = array() ) {
+ $loading_page_path = $this->wp_apply_filters( 'wooblueprint_export_landingpage', '/' );
+ /**
+ * Validate that the landing page path is a valid relative local URL path.
+ *
+ * Accepts:
+ * - /
+ * - /path/to/page
+ *
+ * Rejects:
+ * - http://example.com/path/to/page
+ * - invalid-path
+ */
+ if ( ! preg_match( '#^/$|^/[^/].*#', $loading_page_path ) ) {
+ return new WP_Error( 'wooblueprint_invalid_landing_page_path', 'Invalid loading page path.' );
+ }
+
$schema = array(
- 'landingPage' => $this->wp_apply_filters( 'wooblueprint_export_landingpage', '/' ),
+ 'landingPage' => $loading_page_path,
'steps' => array(),
);
@@ -60,6 +75,14 @@ class ExportSchema {
*/
$exporters = $this->wp_apply_filters( 'wooblueprint_exporters', array_merge( $this->exporters, $built_in_exporters ) );
+ // Validate that the exporters are instances of StepExporter.
+ $exporters = array_filter(
+ $exporters,
+ function ( $exporter ) {
+ return $exporter instanceof StepExporter;
+ }
+ );
+
// Filter out any exporters that are not in the list of steps to export.
if ( count( $steps ) ) {
foreach ( $exporters as $key => $exporter ) {
diff --git a/packages/php/blueprint/src/ImportSchema.php b/packages/php/blueprint/src/ImportSchema.php
index 7e0ba71ab7..f1c913a7a7 100644
--- a/packages/php/blueprint/src/ImportSchema.php
+++ b/packages/php/blueprint/src/ImportSchema.php
@@ -68,11 +68,11 @@ class ImportSchema {
*
* @param string $file The file path.
* @return ImportSchema The created ImportSchema instance.
+ *
+ * @throws \RuntimeException If the JSON file cannot be read.
+ * @throws \InvalidArgumentException If the JSON is invalid or missing 'steps' field.
*/
public static function create_from_file( $file ) {
- // @todo check for mime type
- $path_info = pathinfo( $file );
-
return self::create_from_json( $file );
}
@@ -81,6 +81,9 @@ class ImportSchema {
*
* @param string $json_path The JSON file path.
* @return ImportSchema The created ImportSchema instance.
+ *
+ * @throws \RuntimeException If the JSON file cannot be read.
+ * @throws \InvalidArgumentException If the JSON is invalid or missing 'steps' field.
*/
public static function create_from_json( $json_path ) {
return new self( new JsonSchema( $json_path ) );
@@ -109,6 +112,14 @@ class ImportSchema {
*/
$step_processors = $this->wp_apply_filters( 'wooblueprint_importers', $step_processors );
+ // Validate that the step processors are instances of StepProcessor.
+ $step_processors = array_filter(
+ $step_processors,
+ function ( $step_processor ) {
+ return $step_processor instanceof StepProcessor;
+ }
+ );
+
$indexed_step_processors = Util::index_array(
$step_processors,
function ( $key, $step_processor ) {
@@ -159,8 +170,7 @@ class ImportSchema {
continue;
}
- // phpcs:ignore
- $validate = $this->validator->validate( $step_json, json_encode( $step_schema ) );
+ $validate = $this->validator->validate( $step_json, wp_json_encode( $step_schema ) );
if ( ! $validate->isValid() ) {
$result->add_error( "Schema validation failed for step {$step_json->step}" );
diff --git a/packages/php/blueprint/src/ImportStep.php b/packages/php/blueprint/src/ImportStep.php
index bb8fbc14ff..65b1af972a 100644
--- a/packages/php/blueprint/src/ImportStep.php
+++ b/packages/php/blueprint/src/ImportStep.php
@@ -87,6 +87,12 @@ class ImportStep {
$importer = $this->indexed_importers[ $this->step_definition->step ];
+ // validate importer is a step processor before processing.
+ if ( ! $importer instanceof StepProcessor ) {
+ $result->add_warn( "Importer {$this->step_definition->step} is not a valid step processor" );
+ return $result;
+ }
+
// validate steps before processing.
$this->validate_step_schemas( $importer, $result );
diff --git a/packages/php/blueprint/tests/Unit/ExportSchemaTest.php b/packages/php/blueprint/tests/Unit/ExportSchemaTest.php
index 1180a066e8..68a0179868 100644
--- a/packages/php/blueprint/tests/Unit/ExportSchemaTest.php
+++ b/packages/php/blueprint/tests/Unit/ExportSchemaTest.php
@@ -7,7 +7,7 @@ use Automattic\WooCommerce\Blueprint\ExportSchema;
use Automattic\WooCommerce\Blueprint\Tests\stubs\Exporters\EmptySetSiteOptionsExporter;
use Automattic\WooCommerce\Blueprint\Tests\TestCase;
use Mockery;
-use Mockery\Mock;
+use WP_Error;
/**
* Class ExportSchemaTest
@@ -63,10 +63,23 @@ class ExportSchemaTest extends TestCase {
$exporter->shouldReceive( 'wp_apply_filters' )
->with( 'wooblueprint_export_landingpage', Mockery::any() )
- ->andReturn( 'test' );
+ ->andReturn( '/test' );
$result = $exporter->export();
- $this->assertEquals( 'test', $result['landingPage'] );
+ $this->assertEquals( '/test', $result['landingPage'] );
+ }
+
+ /**
+ * Test that it returns a WP_Error when the landing page path is invalid.
+ */
+ public function test_returns_wp_error_when_landing_page_path_is_invalid() {
+ $exporter = $this->get_mock( true );
+ $exporter->shouldReceive( 'wp_apply_filters' )
+ ->with( 'wooblueprint_export_landingpage', Mockery::any() )
+ ->andReturn( 'invalid-path' );
+
+ $result = $exporter->export();
+ $this->assertInstanceOf( WP_Error::class, $result );
}
/**
@@ -94,4 +107,45 @@ class ExportSchemaTest extends TestCase {
$this->assertCount( 1, $result['steps'] );
$this->assertEquals( 'setSiteOptions', $result['steps'][0]['step'] );
}
+
+ /**
+ * Test that it filters out exporters that are not instances of StepExporter.
+ *
+ * @return void
+ */
+ public function test_it_filters_out_invalid_exporters() {
+ $empty_exporter = new EmptySetSiteOptionsExporter();
+ $invalid_exporter = new class() {
+ /**
+ * Export method that should never be called.
+ *
+ * @throws \Exception If called.
+ */
+ public function export() {
+ throw new \Exception( 'This method should not be called.' );
+ }
+ };
+
+ $mock = Mock(
+ ExportSchema::class,
+ array(
+ array(
+ $empty_exporter,
+ $invalid_exporter,
+ ),
+ )
+ );
+ $mock->makePartial();
+
+ // Mock the filter to return our test exporters.
+ $mock->shouldReceive( 'wp_apply_filters' )
+ ->with( 'wooblueprint_exporters', Mockery::any() )
+ ->andReturn( array( $empty_exporter, $invalid_exporter ) );
+
+ $result = $mock->export();
+
+ // Should only have one step from the valid exporter.
+ $this->assertCount( 1, $result['steps'] );
+ $this->assertEquals( 'setSiteOptions', $result['steps'][0]['step'] );
+ }
}
diff --git a/packages/php/blueprint/tests/Unit/ImportStepTest.php b/packages/php/blueprint/tests/Unit/ImportStepTest.php
index f79c7b7867..ad2714f662 100644
--- a/packages/php/blueprint/tests/Unit/ImportStepTest.php
+++ b/packages/php/blueprint/tests/Unit/ImportStepTest.php
@@ -1,6 +1,7 @@
<?php
use Automattic\WooCommerce\Blueprint\Tests\stubs\Importers\DummyImporter;
+use Automattic\WooCommerce\Blueprint\Tests\stubs\Steps\DummyStep;
use PHPUnit\Framework\TestCase;
use Automattic\WooCommerce\Blueprint\ImportStep;
use Automattic\WooCommerce\Blueprint\StepProcessorResult;
@@ -59,6 +60,37 @@ class ImportStepTest extends TestCase {
$this->assertEquals( 'Unable to find an importer for dummy' . $rand, $result->get_messages( 'warn' )[0]['message'] );
}
+ /**
+ * Test it returns warn when importer is not a step processor.
+ *
+ * @return void
+ */
+ public function test_it_returns_warn_when_importer_is_not_a_step_processor() {
+ // Create a filter that adds an invalid importer (not implementing StepProcessor).
+ add_filter(
+ 'wooblueprint_importers',
+ function ( $importers ) {
+ $importers[] = new class() {
+ /**
+ * Get the step class name.
+ *
+ * @return string
+ */
+ public function get_step_class() {
+ return DummyStep::class;
+ }
+ };
+ return $importers;
+ }
+ );
+
+ $importer = new ImportStep( (object) array( 'step' => DummyStep::get_step_name() ) );
+ $result = $importer->import();
+
+ $this->assertCount( 1, $result->get_messages( 'warn' ) );
+ $this->assertEquals( sprintf( 'Importer %s is not a valid step processor', DummyStep::get_step_name() ), $result->get_messages( 'warn' )[0]['message'] );
+ }
+
/**
* Test it returns validation error.
*