From 8a315155ec70883fc94c09444cf430ed5c6ed2b3 Mon Sep 17 00:00:00 2001 From: girishpanchal30 Date: Thu, 10 Sep 2026 11:40:52 +0530 Subject: [PATCH 1/3] fix: sanitize player properties --- inc/video_player.php | 125 +++++++++++++++++++---- tests/test-video-player-block.php | 161 ++++++++++++++++++++++++++++++ 2 files changed, 269 insertions(+), 17 deletions(-) create mode 100644 tests/test-video-player-block.php diff --git a/inc/video_player.php b/inc/video_player.php index ea65b1b6..c9a54d34 100644 --- a/inc/video_player.php +++ b/inc/video_player.php @@ -15,6 +15,7 @@ class Optml_Video_Player { private $block_attributes = [ 'url' => [ 'type' => 'string', + 'default' => '', ], 'primaryColor' => [ 'type' => 'string', @@ -197,11 +198,11 @@ public function register_video_player_block() { * @since 4.0.0 */ public function render_video_player_block( $attributes, $content, $block ) { - $attributes = wp_parse_args( $attributes, $this->block_attributes ); + $attributes = wp_parse_args( $attributes, $this->get_default_attributes() ); $style = [ - '--om-primary-color' => $attributes['primaryColor'], - '--om-aspect-ratio' => $attributes['aspectRatio'], + '--om-primary-color' => $this->sanitize_primary_color( $attributes['primaryColor'] ), + '--om-aspect-ratio' => $this->sanitize_aspect_ratio( $attributes['aspectRatio'] ), ]; if ( isset( $attributes['style'] ) ) { @@ -231,17 +232,10 @@ function ( $key, $value ) { $tag_attributes ); - $wrapper_attributes = array_filter( - $attributes, - function ( $key ) { - return ! in_array( $key, array_keys( $this->block_attributes ), true ) && $key !== 'style'; - }, - ARRAY_FILTER_USE_KEY - ); - + // Alignment and custom classes already come from block supports, so no attribute is forwarded here. return sprintf( '
', - get_block_wrapper_attributes( $wrapper_attributes ), + get_block_wrapper_attributes(), implode( ' ', $tag_attributes ), ); } @@ -303,19 +297,116 @@ private function get_localization( $editor = false ) { private function block_style_attributes_to_css_array( $attributes ) { $css = []; - if ( isset( $attributes['spacing'] ) ) { - $spacing = $attributes['spacing']; + if ( ! isset( $attributes['spacing'] ) || ! is_array( $attributes['spacing'] ) ) { + return $css; + } + + $allowed_props = [ 'margin', 'padding' ]; + $allowed_directions = [ 'top', 'right', 'bottom', 'left' ]; + + foreach ( $attributes['spacing'] as $css_prop_prefix => $values ) { + if ( ! in_array( $css_prop_prefix, $allowed_props, true ) || ! is_array( $values ) ) { + continue; + } - foreach ( $spacing as $css_prop_prefix => $values ) { - foreach ( $values as $direction => $value ) { - $css[ $css_prop_prefix . '-' . $direction ] = $this->core_var_to_css_var( $value ); + foreach ( $values as $direction => $value ) { + if ( ! in_array( $direction, $allowed_directions, true ) || ! is_string( $value ) ) { + continue; } + + $value = $this->core_var_to_css_var( $value ); + + if ( ! $this->is_safe_css_length( $value ) ) { + continue; + } + + $css[ $css_prop_prefix . '-' . $direction ] = $value; } } return $css; } + /** + * Get the default value of every declared block attribute. + * + * @return array The default attributes. + */ + private function get_default_attributes() { + $defaults = []; + + foreach ( $this->block_attributes as $name => $schema ) { + if ( ! isset( $schema['default'] ) ) { + continue; + } + + $defaults[ $name ] = $schema['default']; + } + + return $defaults; + } + + /** + * Sanitize the player primary color, falling back to the default when it is not a css color. + * + * @param mixed $color The color to sanitize. + * @return string The sanitized color. + */ + private function sanitize_primary_color( $color ) { + $default = $this->block_attributes['primaryColor']['default']; + + if ( ! is_string( $color ) ) { + return $default; + } + + $color = trim( $color ); + + $allowed = [ + '/^#(?:[0-9a-f]{3,4}|[0-9a-f]{6}|[0-9a-f]{8})$/i', // Hex. + '/^[a-z]+$/i', // Named color. + '/^(?:rgb|hsl)a?\(\s*[0-9a-z.%,\/\s-]+\)$/i', // Functional notation. + '/^var\(\s*--[a-z0-9-]+\s*\)$/i', // Theme preset. + ]; + + foreach ( $allowed as $pattern ) { + if ( preg_match( $pattern, $color ) ) { + return $color; + } + } + + return $default; + } + + /** + * Sanitize the player aspect ratio, falling back to the default when it is not a known one. + * + * @param mixed $aspect_ratio The aspect ratio to sanitize. + * @return string The sanitized aspect ratio. + */ + private function sanitize_aspect_ratio( $aspect_ratio ) { + $allowed = [ 'auto', '16/9', '4/3', '1/1', '9/16', '1/2', '2/1' ]; + + if ( is_string( $aspect_ratio ) && in_array( $aspect_ratio, $allowed, true ) ) { + return $aspect_ratio; + } + + return $this->block_attributes['aspectRatio']['default']; + } + + /** + * Whether a value is a css length or a core preset variable. + * + * @param string $value The value to check. + * @return bool Whether the value is safe to use as a css length. + */ + private function is_safe_css_length( $value ) { + if ( preg_match( '/^var\(--wp--[a-z0-9-]+\)$/i', $value ) ) { + return true; + } + + return (bool) preg_match( '/^-?(?:\d+|\d*\.\d+)(?:px|em|rem|%|vh|vw|vmin|vmax|ch|ex|pt|pc|cm|mm|in)?$/i', $value ); + } + /** * Convert a core var to a css var. * e.g.: var:preset|spacing|50 -> var(--wp--preset--spacing--50) diff --git a/tests/test-video-player-block.php b/tests/test-video-player-block.php new file mode 100644 index 00000000..a6b63db4 --- /dev/null +++ b/tests/test-video-player-block.php @@ -0,0 +1,161 @@ +update( 'service_data', [ + 'cdn_key' => 'test123', + 'cdn_secret' => '12345', + 'whitelist' => [ 'example.com' ], + ] ); + + if ( ! WP_Block_Type_Registry::get_instance()->is_registered( 'optimole/video-player' ) ) { + $player = new Optml_Video_Player(); + $player->register_video_player_block(); + $this->registered_block = true; + } + } + + public function tearDown(): void { + if ( $this->registered_block ) { + unregister_block_type( 'optimole/video-player' ); + $this->registered_block = false; + } + + parent::tearDown(); + } + + /** + * Render a serialized video player block. + * + * @param array $attributes The block attributes. + * @return string The rendered markup. + */ + private function render( $attributes ) { + return do_blocks( '' ); + } + + /** + * Undeclared attributes must never reach the wrapper element. + */ + public function test_event_handler_attributes_are_not_rendered() { + $rendered = $this->render( [ + 'url' => 'https://example.com/video.mp4', + 'aspectRatio' => '16/9', + 'onmouseover' => 'alert(document.domain)', + 'onerror' => 'alert(1)', + 'onfocus' => 'alert(2)', + 'data-wp-on--click' => 'actions.evil', + ] ); + + $this->assertStringNotContainsString( 'onmouseover', $rendered ); + $this->assertStringNotContainsString( 'onerror', $rendered ); + $this->assertStringNotContainsString( 'onfocus', $rendered ); + $this->assertStringNotContainsString( 'data-wp-on', $rendered ); + $this->assertStringNotContainsString( 'alert(', $rendered ); + } + + /** + * The wrapper keeps the classes produced by block supports. + */ + public function test_block_support_classes_are_preserved() { + $rendered = $this->render( [ + 'url' => 'https://example.com/video.mp4', + 'align' => 'wide', + 'className' => 'my-custom-class', + ] ); + + $this->assertStringContainsString( 'wp-block-optimole-video-player', $rendered ); + $this->assertStringContainsString( 'alignwide', $rendered ); + $this->assertStringContainsString( 'my-custom-class', $rendered ); + } + + /** + * The player element keeps rendering its own attributes. + */ + public function test_player_element_attributes_are_rendered() { + $rendered = $this->render( [ + 'url' => 'https://example.com/video.mp4', + 'aspectRatio' => '4/3', + 'primaryColor' => '#ff0000', + 'loop' => true, + 'hideControls' => true, + ] ); + + $this->assertStringContainsString( 'video-src="https://example.com/video.mp4"', $rendered ); + $this->assertStringContainsString( 'loop="true"', $rendered ); + $this->assertStringContainsString( 'hide-controls="true"', $rendered ); + $this->assertStringContainsString( '--om-primary-color: #ff0000', $rendered ); + $this->assertStringContainsString( '--om-aspect-ratio: 4/3', $rendered ); + } + + /** + * A block saved without a url renders instead of fataling. + */ + public function test_block_without_url_renders() { + $rendered = do_blocks( '' ); + + $this->assertStringContainsString( 'assertStringContainsString( 'video-src=""', $rendered ); + $this->assertStringNotContainsString( 'Array', $rendered ); + } + + /** + * Style values that are not colors or known ratios fall back to the defaults. + */ + public function test_style_values_are_sanitized() { + $rendered = $this->render( [ + 'url' => 'https://example.com/video.mp4', + 'aspectRatio' => 'auto;background:url(https://evil.test/a)', + 'primaryColor' => 'red;position:fixed;top:0', + ] ); + + $this->assertStringNotContainsString( 'evil.test', $rendered ); + $this->assertStringNotContainsString( 'position:fixed', $rendered ); + $this->assertStringContainsString( '--om-primary-color: #577BF9', $rendered ); + $this->assertStringContainsString( '--om-aspect-ratio: auto', $rendered ); + } + + /** + * Spacing styles only render for known properties, directions and lengths. + */ + public function test_spacing_styles_are_sanitized() { + $rendered = $this->render( [ + 'url' => 'https://example.com/video.mp4', + 'style' => [ + 'spacing' => [ + 'margin' => [ + 'top' => 'var:preset|spacing|50', + 'bottom' => '10px', + 'left' => '0;background:url(https://evil.test/b)', + 'right;color:red' => '5px', + ], + 'behavior;color:blue' => [ + 'top' => '5px', + ], + ], + ], + ] ); + + $this->assertStringContainsString( 'margin-top: var(--wp--preset--spacing--50)', $rendered ); + $this->assertStringContainsString( 'margin-bottom: 10px', $rendered ); + $this->assertStringNotContainsString( 'evil.test', $rendered ); + $this->assertStringNotContainsString( 'color:red', $rendered ); + $this->assertStringNotContainsString( 'color:blue', $rendered ); + } +} From c3b118d71334373fd9dca513f821cc5f57bca514 Mon Sep 17 00:00:00 2001 From: girishpanchal30 Date: Thu, 10 Sep 2026 14:52:14 +0530 Subject: [PATCH 2/3] fix: revert primary color handling --- inc/video_player.php | 33 +------------------------------ tests/test-video-player-block.php | 3 +-- 2 files changed, 2 insertions(+), 34 deletions(-) diff --git a/inc/video_player.php b/inc/video_player.php index c9a54d34..9374b605 100644 --- a/inc/video_player.php +++ b/inc/video_player.php @@ -201,7 +201,7 @@ public function render_video_player_block( $attributes, $content, $block ) { $attributes = wp_parse_args( $attributes, $this->get_default_attributes() ); $style = [ - '--om-primary-color' => $this->sanitize_primary_color( $attributes['primaryColor'] ), + '--om-primary-color' => $attributes['primaryColor'], '--om-aspect-ratio' => $this->sanitize_aspect_ratio( $attributes['aspectRatio'] ), ]; @@ -346,37 +346,6 @@ private function get_default_attributes() { return $defaults; } - /** - * Sanitize the player primary color, falling back to the default when it is not a css color. - * - * @param mixed $color The color to sanitize. - * @return string The sanitized color. - */ - private function sanitize_primary_color( $color ) { - $default = $this->block_attributes['primaryColor']['default']; - - if ( ! is_string( $color ) ) { - return $default; - } - - $color = trim( $color ); - - $allowed = [ - '/^#(?:[0-9a-f]{3,4}|[0-9a-f]{6}|[0-9a-f]{8})$/i', // Hex. - '/^[a-z]+$/i', // Named color. - '/^(?:rgb|hsl)a?\(\s*[0-9a-z.%,\/\s-]+\)$/i', // Functional notation. - '/^var\(\s*--[a-z0-9-]+\s*\)$/i', // Theme preset. - ]; - - foreach ( $allowed as $pattern ) { - if ( preg_match( $pattern, $color ) ) { - return $color; - } - } - - return $default; - } - /** * Sanitize the player aspect ratio, falling back to the default when it is not a known one. * diff --git a/tests/test-video-player-block.php b/tests/test-video-player-block.php index a6b63db4..83907830 100644 --- a/tests/test-video-player-block.php +++ b/tests/test-video-player-block.php @@ -126,8 +126,7 @@ public function test_style_values_are_sanitized() { ] ); $this->assertStringNotContainsString( 'evil.test', $rendered ); - $this->assertStringNotContainsString( 'position:fixed', $rendered ); - $this->assertStringContainsString( '--om-primary-color: #577BF9', $rendered ); + $this->assertStringContainsString( '--om-primary-color: red;position:fixed;top:0', $rendered ); $this->assertStringContainsString( '--om-aspect-ratio: auto', $rendered ); } From bd45a7f0e48e2dc8b36d987937449b9238aa02ab Mon Sep 17 00:00:00 2001 From: girishpanchal30 Date: Fri, 11 Sep 2026 13:48:18 +0530 Subject: [PATCH 3/3] fix: accept all wordpress spacing units --- inc/video_player.php | 3 ++- tests/test-video-player-block.php | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/inc/video_player.php b/inc/video_player.php index 9374b605..ee097116 100644 --- a/inc/video_player.php +++ b/inc/video_player.php @@ -373,7 +373,8 @@ private function is_safe_css_length( $value ) { return true; } - return (bool) preg_match( '/^-?(?:\d+|\d*\.\d+)(?:px|em|rem|%|vh|vw|vmin|vmax|ch|ex|pt|pc|cm|mm|in)?$/i', $value ); + // Number plus an optional unit; no css separator can pass this shape. + return (bool) preg_match( '/^-?(?:\d+|\d*\.\d+)(?:%|[a-z]{1,6})?$/i', $value ); } /** diff --git a/tests/test-video-player-block.php b/tests/test-video-player-block.php index 83907830..9150f63c 100644 --- a/tests/test-video-player-block.php +++ b/tests/test-video-player-block.php @@ -130,6 +130,28 @@ public function test_style_values_are_sanitized() { $this->assertStringContainsString( '--om-aspect-ratio: auto', $rendered ); } + /** + * Every unit WordPress's spacing control can offer survives sanitization. + */ + public function test_spacing_accepts_all_wordpress_units() { + $units = [ + '%', 'px', 'em', 'rem', 'ch', 'ex', 'cm', 'mm', 'in', 'pt', 'pc', + 'vw', 'vh', 'vmin', 'vmax', + 'svw', 'svh', 'svi', 'svb', 'svmin', 'svmax', + 'lvw', 'lvh', 'lvi', 'lvb', 'lvmin', 'lvmax', + 'dvw', 'dvh', 'dvi', 'dvb', 'dvmin', 'dvmax', + ]; + + foreach ( $units as $unit ) { + $rendered = $this->render( [ + 'url' => 'https://example.com/video.mp4', + 'style' => [ 'spacing' => [ 'margin' => [ 'top' => '10' . $unit ] ] ], + ] ); + + $this->assertStringContainsString( 'margin-top: 10' . $unit, $rendered, '10' . $unit . ' should be preserved' ); + } + } + /** * Spacing styles only render for known properties, directions and lengths. */