diff --git a/src/DataObject/Storage/CustomPostTypeStorage.php b/src/DataObject/Storage/CustomPostTypeStorage.php index 26f1703..1ab0faf 100644 --- a/src/DataObject/Storage/CustomPostTypeStorage.php +++ b/src/DataObject/Storage/CustomPostTypeStorage.php @@ -21,14 +21,14 @@ public function insert( array $data ): int { $post_id = wp_insert_post( [ 'post_type' => $this->slug, 'post_status' => 'publish', - 'post_title' => $data['title'] ?? '', + 'post_title' => $this->prepare_data( $data['title'] ?? '' ), ], true ); if ( is_wp_error( $post_id ) ) { return 0; } - update_post_meta( $post_id, self::META_KEY, wp_json_encode( $data ) ); + update_post_meta( $post_id, self::META_KEY, $this->prepare_data( $data ) ); return $post_id; } @@ -42,11 +42,11 @@ public function update( int $id, array $data ): void { if ( isset( $data['title'] ) ) { wp_update_post( [ 'ID' => $id, - 'post_title' => $data['title'], + 'post_title' => $this->prepare_data( $data['title'] ), ] ); } - update_post_meta( $id, self::META_KEY, wp_json_encode( $data ) ); + update_post_meta( $id, self::META_KEY, $this->prepare_data( $data ) ); } public function delete( int $id ): void { @@ -89,4 +89,19 @@ public function all(): array { return $results; } + + /** + * We expect $value to be unslashed, as it will either come + * from Request::get_body_params() or be manually set + * + * Post and meta functions both expect slashed data: + * @see https://developer.wordpress.org/reference/hooks/wp_insert_post_data/ + * @see https://developer.wordpress.org/reference/functions/update_post_meta/ + */ + protected function prepare_data( string|array $value ): string { + if ( is_array( $value ) ) { + $value = wp_json_encode( $value ); + } + return wp_slash( $value ); + } } diff --git a/src/DataView/FieldTypeRegistry.php b/src/DataView/FieldTypeRegistry.php index f62e196..cdaf31e 100644 --- a/src/DataView/FieldTypeRegistry.php +++ b/src/DataView/FieldTypeRegistry.php @@ -209,7 +209,7 @@ public function sanitize_boolean( mixed $value ): bool { public function sanitize_repeater( mixed $value ): string { // Handle JSON string input (from Tangible Fields hidden input). if ( is_string( $value ) ) { - $decoded = json_decode( stripslashes( $value ), true ); + $decoded = json_decode( $value, true ); if ( json_last_error() === JSON_ERROR_NONE && is_array( $decoded ) ) { return wp_json_encode( $this->sanitize_repeater_rows( $decoded ) ); } diff --git a/src/DataView/Request.php b/src/DataView/Request.php index aa6b16e..7304577 100644 --- a/src/DataView/Request.php +++ b/src/DataView/Request.php @@ -82,4 +82,11 @@ public function get_nonce( ?string $name = '_wpnonce' ): string { public function is_post(): bool { return $this->rest_request->is_method( 'POST' ); } + + /** + * Get the POST body parameters (unslashed) + */ + public function get_body_params(): array { + return $this->rest_request->get_body_params(); + } } diff --git a/src/DataView/RequestRouter.php b/src/DataView/RequestRouter.php index 29ef46e..eb3804f 100644 --- a/src/DataView/RequestRouter.php +++ b/src/DataView/RequestRouter.php @@ -10,7 +10,6 @@ use Tangible\Renderer\HtmlRenderer; use Tangible\RequestHandler\PluralHandler; use Tangible\RequestHandler\SingularHandler; -use Tangible\RequestHandler\Result; /** * Handles request routing and rendering for DataView admin pages. @@ -441,15 +440,14 @@ protected function handle_settings_submit(): void { protected function extract_post_data(): array { $data = []; - // phpcs:ignore WordPress.Security.NonceVerification.Missing - $nested = $_POST[ $this->config->slug ] ?? []; + $params = $this->request->get_body_params(); + $nested = $params[ $this->config->slug ] ?? []; foreach ( $this->config->field_configs as $name => $config ) { $type = $config['type']; // Check nested array first (singular/settings mode), then flat POST - // phpcs:ignore WordPress.Security.NonceVerification.Missing - $has_value = isset( $nested[ $name ] ) || isset( $_POST[ $name ] ); + $has_value = isset( $nested[ $name ] ) || isset( $params[ $name ] ); if ( ! $has_value ) { // Handle missing boolean fields (unchecked checkboxes). @@ -464,8 +462,7 @@ protected function extract_post_data(): array { } $sanitizer = $this->registry->get_sanitizer( $type ); - // phpcs:ignore WordPress.Security.NonceVerification.Missing - $raw_value = $nested[ $name ] ?? $_POST[ $name ]; + $raw_value = $nested[ $name ] ?? $params[ $name ]; $data[ $name ] = $sanitizer( $raw_value ); } diff --git a/tests/phpunit/data-view.php b/tests/phpunit/data-view.php index b0629b7..f5eb464 100644 --- a/tests/phpunit/data-view.php +++ b/tests/phpunit/data-view.php @@ -487,20 +487,6 @@ public function test_repeater_sanitizer_handles_json_string(): void { $this->assertEquals( 5, $decoded[0]['count'] ); } - public function test_repeater_sanitizer_handles_escaped_json(): void { - $registry = new FieldTypeRegistry(); - $sanitizer = $registry->get_sanitizer( 'repeater' ); - - // Simulates WordPress POST data with escaped quotes. - $input = addslashes( '[{"key":"abc","name":"Test"}]' ); - $result = $sanitizer( $input ); - $decoded = json_decode( $result, true ); - - $this->assertIsArray( $decoded ); - $this->assertCount( 1, $decoded ); - $this->assertEquals( 'Test', $decoded[0]['name'] ); - } - public function test_repeater_sanitizer_handles_array_input(): void { $registry = new FieldTypeRegistry(); $sanitizer = $registry->get_sanitizer( 'repeater' ); diff --git a/tests/phpunit/save-data.php b/tests/phpunit/save-data.php new file mode 100644 index 0000000..949bfdb --- /dev/null +++ b/tests/phpunit/save-data.php @@ -0,0 +1,289 @@ +factory->user->create( [ 'role' => 'administrator' ] ); + wp_set_current_user( $administrator ); + + if ( function_exists( 'tangible_fields' ) ) { + tangible_fields()->registered_fields = []; + } + } + + /** + * ========================================================================== + * Cases + * ========================================================================== + */ + + /** + * Shared by the plural and single entity providers, so both modes cover + * the same values + * -> field type + * -> submitted value + * -> expected stored value + */ + private function cases(): array { + return [ + + 'string' => [ + 'string', + self::HOSTILE_STRING, + self::HOSTILE_STRING, + ], + + 'xss attempt' => [ + 'string', + 'hello world', + 'hello world', + ], + + 'html tags' => [ + 'string', + '
hello
world', + 'hello world', + ], + + 'boolean' => [ + 'boolean', + '1', + true, + ], + + 'map' => [ + 'map', + [ 'nested' => [ 'path' => self::HOSTILE_STRING ] ], + [ 'nested' => [ 'path' => self::HOSTILE_STRING ] ], + ], + + 'repeater' => [ + [ 'type' => 'repeater', 'sub_fields' => [ [ 'name' => 'label', 'type' => 'string' ] ] ], + wp_json_encode( [ [ 'label' => self::HOSTILE_STRING ] ] ), + wp_json_encode( [ [ 'label' => self::HOSTILE_STRING ] ] ), + ], + ]; + } + + /** + * ========================================================================== + * Helpers + * ========================================================================== + */ + + /** + * Reach the protected RequestRouter on a DataView for direct invocation. + */ + private function get_router( DataView $view ): \Tangible\DataView\RequestRouter { + $property = new \ReflectionProperty( DataView::class, 'router' ); + $property->setAccessible( true ); + return $property->getValue( $view ); + } + + /** + * Build the DataView on the staged request and fire maybe_redirect() + * (the admin_init entry point). + * + * A successful submit ends in wp_redirect() + exit: the wp_redirect filter + * runs first, so throwing there keeps PHPUnit alive and captures the location. + * + * @return array{0: DataView, 1: string} The view and the redirect location. + */ + private function submit_request( array $config ): array { + $_SERVER['REQUEST_METHOD'] = 'POST'; + + add_filter( 'wp_redirect', static function ( string $location ): string { + throw new \Exception( $location ); + } ); + + try { + $view = new DataView( $config ); + $this->get_router( $view )->maybe_redirect(); + } catch ( \Exception $e ) { + return [ $view, $e->getMessage() ]; + } + + $this->fail( 'Expected the success redirect after submit' ); + } + + /** + * ========================================================================== + * Plural entity + * ========================================================================== + */ + + public function plural_submit_round_trip_provider(): array { + /** + * Database storage has no array columns (map), and returns + * booleans as the string "1" since nothing coerces on read + */ + $unsupported = [ + 'database' => [ 'map', 'boolean' ], + ]; + + $rows = []; + foreach ( [ 'cpt', 'database' ] as $storage ) { + foreach ( $this->cases() as $name => $case ) { + if ( in_array( $name, $unsupported[ $storage ] ?? [], true ) ) { + continue; + } + $rows[ "{$storage}: {$name}" ] = [ $storage, ...$case ]; + } + } + + return $rows; + } + + /** + * @dataProvider plural_submit_round_trip_provider + */ + public function test_plural_submit_round_trips_value( + string $storage, + string|array $field_type, + string|array $submitted, + mixed $expected + ): void { + if ( $storage === 'database' && ! function_exists( 'tdb_register_table' ) ) { + $this->markTestSkipped( 'Database module (TDB) is not loaded.' ); + } + + $type_slug = is_array( $field_type ) ? $field_type['type'] : $field_type; + $slug = "dv_e2e_{$storage}_{$type_slug}"; + $config = [ + 'slug' => $slug, + 'label' => 'Item', + 'storage' => $storage, + 'fields' => [ + 'title' => 'string', + 'value' => $field_type, + ], + ]; + + $id = ( new DataView( $config ) ) + ->get_handler() + ->create( ['title' => 'before', 'value' => 'before' ] ) + ->get_entity() + ->get_id(); + + $_GET['page'] = $slug; + $_GET['action'] = 'edit'; + $_GET['id'] = (string) $id; + $_POST['title'] = wp_slash( self::HOSTILE_STRING ); + $_POST['value'] = wp_slash( $submitted ); + $_POST['_wpnonce_edit'] = wp_create_nonce( "{$slug}_edit_{$id}" ); + + [ $view, $location ] = $this->submit_request( $config ); + + $this->assertStringContainsString( 'updated=1', $location ); + + $entity = $view->get_handler()->read( $id )->get_entity(); + $this->assertSame( $expected, $entity->get( 'value' ) ); + $this->assertSame( self::HOSTILE_STRING, $entity->get( 'title' ) ); + + // CPT storage also writes the title as post_title + if ( $storage === 'cpt' ) { + $this->assertSame( self::HOSTILE_STRING, get_post( $id )->post_title ); + } + } + + public function test_plural_create_submit_round_trips_value(): void { + $config = [ + 'slug' => 'dv_e2e_create', + 'label' => 'Item', + 'fields' => [ 'title' => 'string' ], + ]; + + $_GET['page'] = 'dv_e2e_create'; + $_GET['action'] = 'create'; + $_POST['title'] = wp_slash( self::HOSTILE_STRING ); + $_POST['_wpnonce_create'] = wp_create_nonce( 'dv_e2e_create_create' ); + + [ $view, $location ] = $this->submit_request( $config ); + + $this->assertStringContainsString( 'created=1', $location ); + + // The redirect targets the new entity's edit page + parse_str( (string) parse_url( $location, PHP_URL_QUERY ), $query ); + $id = (int) $query['id']; + + $this->assertSame( + self::HOSTILE_STRING, + $view + ->get_handler() + ->read( $id ) + ->get_entity() + ->get( 'title' ) + ); + + $this->assertSame( + self::HOSTILE_STRING, + get_post( $id )->post_title + ); + } + + /** + * ========================================================================== + * Single entity + * ========================================================================== + */ + + public function singular_submit_round_trip_provider(): array { + return $this->cases(); + } + + /** + * @dataProvider singular_submit_round_trip_provider + */ + public function test_singular_submit_round_trips_value( + string|array $field_type, + string|array $submitted, + mixed $expected + ): void { + $type_slug = is_array( $field_type ) ? $field_type['type'] : $field_type; + $slug = "dv_e2e_option_{$type_slug}"; + $config = [ + 'slug' => $slug, + 'label' => 'Settings', + 'mode' => 'singular', + 'storage' => 'option', + 'fields' => [ 'value' => $field_type ], + ]; + + $_GET['page'] = $slug; + $_POST['value'] = wp_slash( $submitted ); + $_POST['_wpnonce_update'] = wp_create_nonce( "{$slug}_update" ); + + [ $view, $location ] = $this->submit_request( $config ); + + $this->assertStringContainsString( 'updated=1', $location ); + + $data = $view->get_handler()->read()->get_data(); + $this->assertSame( $expected, $data['value'] ); + } +}