From aa98465368d0060c1adcd800785057e71178cbc3 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Fri, 23 Oct 2026 17:44:57 -0700 Subject: [PATCH 1/4] KSES: Add wp_kses_sanitize_srcset() A srcset value is a comma-separated list of image candidate strings, not a single URL, so passing it through wp_kses_bad_protocol() or esc_url() either misses candidates or corrupts the list. This function segments the value following the HTML specification's srcset parsing algorithm and removes candidates whose URL uses a disallowed protocol. It takes decoded attribute text and returns decoded text, leaving HTML decoding and encoding to the caller, and returns the input string itself when every candidate is allowed. Nothing calls it yet; wiring it into KSES by tag and attribute follows separately. See https://core.trac.wordpress.org/ticket/29807 --- src/wp-includes/kses.php | 91 +++++++++++ .../tests/kses/wpKsesSanitizeSrcset.php | 153 ++++++++++++++++++ 2 files changed, 244 insertions(+) create mode 100644 tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php diff --git a/src/wp-includes/kses.php b/src/wp-includes/kses.php index c68b71fcc1527..764a7baf49113 100644 --- a/src/wp-includes/kses.php +++ b/src/wp-includes/kses.php @@ -3200,6 +3200,97 @@ function wp_kses_bad_protocol_once2( $scheme, $allowed_protocols ) { } } +/** + * Removes image candidates with a disallowed URL protocol from a `srcset` value. + * + * A `srcset` attribute holds a comma-separated list of image candidate strings, + * each a URL optionally followed by whitespace and width or density descriptors, + * for example `small.jpg 480w, large.jpg 1024w`. The value is split into candidates + * following the HTML specification's srcset parsing algorithm, so a comma only + * separates candidates when it ends a URL or its descriptors; a comma inside a URL, + * as produced by image CDN resizers, belongs to the URL. + * + * Each URL is checked with {@see wp_kses_bad_protocol()}. A candidate whose URL that + * check would change is removed in full, descriptors included, because stripping a + * scheme and keeping the rest would rewrite the URL rather than remove it, and + * descriptors left behind would be read as the next candidate's URL. As in + * {@see esc_url()}, a URL starting with `/` cannot carry a scheme and is kept as is. + * + * The value must be decoded attribute text, as returned by + * {@see WP_HTML_Tag_Processor::get_attribute()}, not raw HTML containing character + * references. The returned value is decoded text too; callers writing it back into + * HTML must escape it. When every candidate is allowed, the input is returned + * unchanged. Otherwise the remaining candidates are rejoined with `, `. + * + * @since 7.2.0 + * + * @link https://html.spec.whatwg.org/multipage/images.html#parsing-a-srcset-attribute + * + * @param string $srcset Decoded `srcset` attribute value. + * @param string[] $allowed_protocols Array of allowed URL protocols. + * @return string The value without candidates whose URL uses a disallowed protocol. + */ +function wp_kses_sanitize_srcset( string $srcset, array $allowed_protocols ): string { + $whitespace = " \t\f\r\n"; + $length = strlen( $srcset ); + $at = 0; + $result = ''; + + /* + * Candidates are only copied once one has to be removed. Until then the + * scan allocates nothing but the URLs it checks, and an all-allowed value + * is returned as the very string that was passed in. + * + * On the first removal the scan restarts to rebuild the output; candidates + * before `$checked_until` passed their check in the first pass and are not + * checked again. + */ + $rebuild = false; + $checked_until = 0; + + while ( $at < $length ) { + $at += strspn( $srcset, "{$whitespace},", $at ); + if ( $at >= $length ) { + break; + } + + // The URL is the run of non-whitespace characters, less any commas ending it. + $candidate_at = $at; + $url_length = strcspn( $srcset, $whitespace, $at ); + $url = rtrim( substr( $srcset, $at, $url_length ), ',' ); + $at += $url_length; + + // A URL ended by a comma has no descriptors; otherwise they run to the next comma. + if ( strlen( $url ) === $url_length ) { + $at += strcspn( $srcset, ',', $at ); + $candidate_length = $at - $candidate_at; + } else { + $candidate_length = strlen( $url ); + } + + if ( $rebuild && $candidate_at < $checked_until ) { + $is_allowed = true; + } else { + $is_allowed = '/' === $url[0] || 0 === strcasecmp( wp_kses_bad_protocol( $url, $allowed_protocols ), $url ); + } + + if ( $rebuild ) { + if ( $is_allowed ) { + $result .= ( '' === $result ? '' : ', ' ) . rtrim( substr( $srcset, $candidate_at, $candidate_length ), $whitespace ); + } + continue; + } + + if ( ! $is_allowed ) { + $rebuild = true; + $checked_until = $candidate_at; + $at = 0; + } + } + + return $rebuild ? $result : $srcset; +} + /** * Converts and fixes HTML entities. * diff --git a/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php b/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php new file mode 100644 index 0000000000000..99679fd36fc13 --- /dev/null +++ b/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php @@ -0,0 +1,153 @@ +allowed_protocols = wp_allowed_protocols(); + } + + /** + * @ticket 29807 + * + * @dataProvider data_allowed_values_are_returned_unchanged + * + * @param string $srcset Value whose candidates are all allowed. + */ + public function test_allowed_values_are_returned_unchanged( string $srcset ) { + $this->assertSame( $srcset, wp_kses_sanitize_srcset( $srcset, $this->allowed_protocols ) ); + } + + /** + * @return array + */ + public function data_allowed_values_are_returned_unchanged() { + return array( + 'empty value' => array( '' ), + 'whitespace only' => array( " \t\n" ), + 'single URL, no descriptor' => array( 'image.jpg' ), + 'relative URLs' => array( 'path/to/test.png 1x, path/to/test-2x.png 2x' ), + 'root-relative URLs' => array( '/test.png 1x, /test-2x.png 2x' ), + 'protocol-relative URLs' => array( '//cdn.example.com/img-300.jpg 300w, //cdn.example.com/img-600.jpg 600w' ), + 'absolute URLs with width descriptors' => array( 'https://example.com/small.jpg 480w, https://example.com/medium.jpg 800w, https://example.com/large.jpg 1200w' ), + 'uppercase scheme' => array( 'HTTPS://example.com/a.jpg 1x, Http://example.com/b.jpg 2x' ), + 'decimal density descriptors' => array( 'image-1x.jpg 1x, image-1.5x.jpg 1.5x, image-2x.jpg 2x' ), + 'no descriptor on the last candidate' => array( 'small.jpg 300w, medium.jpg 600w, large.jpg' ), + 'no whitespace after the comma' => array( 'image1.jpg 1x,image2.jpg 2x' ), + 'extra whitespace is preserved' => array( ' image1.jpg 1x , image2.jpg 2x ' ), + 'newlines are preserved' => array( "image1.jpg 1x,\nimage2.jpg 2x" ), + 'commas inside a URL belong to the URL' => array( 'http://localhost/test.png,big 1x, http://localhost/test.png,medium 2x' ), + 'CDN resizer URLs with commas' => array( 'https://resizer.example/cdn-cgi/image/format=auto,onerror=redirect,quality=80,width=412,height=275,dpr=1,fit=crop,gravity=0.5x0.5/https://bucket.example/photo.jpg 412w, https://resizer.example/cdn-cgi/image/format=auto,quality=80,width=824,height=550/https://bucket.example/photo.jpg 824w' ), + 'CDN resizer URL without descriptor' => array( 'https://cdn.example/image/format=auto,quality=80/img.jpg 400w, https://cdn.example/image/format=auto,quality=80/img-large.jpg' ), + 'root-relative URL containing a colon' => array( '/images/a:b.jpg 1x' ), + ); + } + + /** + * @ticket 29807 + * + * @dataProvider data_candidates_with_disallowed_protocols_are_removed + * + * @param string $srcset Value containing at least one disallowed candidate. + * @param string $expected Value with those candidates removed. + */ + public function test_candidates_with_disallowed_protocols_are_removed( string $srcset, string $expected ) { + $this->assertSame( $expected, wp_kses_sanitize_srcset( $srcset, $this->allowed_protocols ) ); + } + + /** + * @return array + */ + public function data_candidates_with_disallowed_protocols_are_removed() { + return array( + 'first candidate' => array( + 'bad://localhost/test.png 1x, http://localhost/test-2x.png 2x', + 'http://localhost/test-2x.png 2x', + ), + 'last candidate' => array( + 'http://localhost/test.png 1x, bad://localhost/test-2x.png 2x', + 'http://localhost/test.png 1x', + ), + 'middle candidate' => array( + 'image-1x.jpg 1x, javascript:alert(1) 1.5x, image-2x.jpg 2x', + 'image-1x.jpg 1x, image-2x.jpg 2x', + ), + 'only candidate' => array( + 'javascript:alert(1) 1x', + '', + ), + 'every candidate' => array( + 'javascript:alert(1) 1x, data:text/html, 2x', + '', + ), + 'two in a row' => array( + 'a.jpg 1x, javascript:alert(1) 2x, vbscript:msgbox(1) 3x, d.jpg 4x', + 'a.jpg 1x, d.jpg 4x', + ), + 'data URL' => array( + 'data:image/png;base64,iVBORw0KGgo= 1x, https://example.com/a.jpg 2x', + 'https://example.com/a.jpg 2x', + ), + 'URL with commas and a disallowed scheme' => array( + 'http://localhost/test.png,big 1x, bad://localhost/test.png,medium 2x', + 'http://localhost/test.png,big 1x', + ), + 'descriptor-less candidate after a comma' => array( + 'safe.jpg, javascript:alert(1)', + 'safe.jpg', + ), + 'comma-joined URLs form one candidate' => array( + 'safe.jpg,javascript:alert(1) 2x', + '', + ), + 'comma attached to an invalid descriptor' => array( + 'a.jpg 2q,javascript:alert(1) 2x', + 'a.jpg 2q', + ), + 'descriptors are not sanitized as URLs' => array( + 'a.jpg javascript:alert(1), b.jpg 2x', + 'a.jpg javascript:alert(1), b.jpg 2x', + ), + 'scheme that empties under wp_kses_bad_protocol' => array( + 'a:b:c:d:e:f:g 100w, safe.jpg 200w', + 'safe.jpg 200w', + ), + 'emptied scheme without descriptor' => array( + 'a:b:c:d:e:f:g, safe.jpg 200w', + 'safe.jpg 200w', + ), + 'separators are normalized once a candidate is removed' => array( + " a.jpg 1x ,\n\tjavascript:alert(1) 2x ,b.jpg 3x ", + 'a.jpg 1x, b.jpg 3x', + ), + 'allowed scheme is kept in its original case' => array( + 'HTTPS://example.com/a.jpg 1x, javascript:alert(1) 2x', + 'HTTPS://example.com/a.jpg 1x', + ), + ); + } + + /** + * @ticket 29807 + */ + public function test_respects_the_given_allowed_protocols() { + $srcset = 'https://example.com/a.jpg 1x, ftp://example.com/b.jpg 2x'; + + $this->assertSame( $srcset, wp_kses_sanitize_srcset( $srcset, array( 'https', 'ftp' ) ) ); + $this->assertSame( 'https://example.com/a.jpg 1x', wp_kses_sanitize_srcset( $srcset, array( 'https' ) ) ); + $this->assertSame( '', wp_kses_sanitize_srcset( $srcset, array() ) ); + } +} From 77ce1374d3ce420de118f450d3f2cc514aa1df27 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Sun, 25 Oct 2026 15:52:25 -0700 Subject: [PATCH 2/4] Tests: Add native types to the wp_kses_sanitize_srcset() tests. Declare return types on the test methods and data providers and a type on the allowed protocols property, as suggested in review. See #29807. --- tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php b/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php index 99679fd36fc13..51a0bf804dd80 100644 --- a/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php +++ b/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php @@ -13,9 +13,9 @@ class Tests_Kses_WpKsesSanitizeSrcset extends WP_UnitTestCase { * * @var string[] */ - protected $allowed_protocols; + protected array $allowed_protocols; - public function set_up() { + public function set_up(): void { parent::set_up(); $this->allowed_protocols = wp_allowed_protocols(); } @@ -27,14 +27,14 @@ public function set_up() { * * @param string $srcset Value whose candidates are all allowed. */ - public function test_allowed_values_are_returned_unchanged( string $srcset ) { + public function test_allowed_values_are_returned_unchanged( string $srcset ): void { $this->assertSame( $srcset, wp_kses_sanitize_srcset( $srcset, $this->allowed_protocols ) ); } /** * @return array */ - public function data_allowed_values_are_returned_unchanged() { + public function data_allowed_values_are_returned_unchanged(): array { return array( 'empty value' => array( '' ), 'whitespace only' => array( " \t\n" ), @@ -64,14 +64,14 @@ public function data_allowed_values_are_returned_unchanged() { * @param string $srcset Value containing at least one disallowed candidate. * @param string $expected Value with those candidates removed. */ - public function test_candidates_with_disallowed_protocols_are_removed( string $srcset, string $expected ) { + public function test_candidates_with_disallowed_protocols_are_removed( string $srcset, string $expected ): void { $this->assertSame( $expected, wp_kses_sanitize_srcset( $srcset, $this->allowed_protocols ) ); } /** * @return array */ - public function data_candidates_with_disallowed_protocols_are_removed() { + public function data_candidates_with_disallowed_protocols_are_removed(): array { return array( 'first candidate' => array( 'bad://localhost/test.png 1x, http://localhost/test-2x.png 2x', @@ -143,7 +143,7 @@ public function data_candidates_with_disallowed_protocols_are_removed() { /** * @ticket 29807 */ - public function test_respects_the_given_allowed_protocols() { + public function test_respects_the_given_allowed_protocols(): void { $srcset = 'https://example.com/a.jpg 1x, ftp://example.com/b.jpg 2x'; $this->assertSame( $srcset, wp_kses_sanitize_srcset( $srcset, array( 'https', 'ftp' ) ) ); From 144732eda8e4e5616406c868aaad020975d8f157 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Sun, 25 Oct 2026 15:52:32 -0700 Subject: [PATCH 3/4] KSES: Keep commas inside srcset descriptor parentheses. The HTML srcset parser treats a comma between `(` and `)` in the descriptors as part of the descriptor, so ending candidates at the first comma let the sanitizer and the browser disagree on the URLs: a value like `a.jpg 1x(,)/?,javascript:alert(1) 2x` came back unchanged, and rebuilding could truncate allowed candidates. Track parentheses while scanning descriptors, and replace the restart and rebuild pass with a single pass that collects kept candidates. See #29807. --- src/wp-includes/kses.php | 60 ++++++++----------- .../tests/kses/wpKsesSanitizeSrcset.php | 14 +++++ 2 files changed, 38 insertions(+), 36 deletions(-) diff --git a/src/wp-includes/kses.php b/src/wp-includes/kses.php index 764a7baf49113..e69cec4d3a204 100644 --- a/src/wp-includes/kses.php +++ b/src/wp-includes/kses.php @@ -3207,8 +3207,9 @@ function wp_kses_bad_protocol_once2( $scheme, $allowed_protocols ) { * each a URL optionally followed by whitespace and width or density descriptors, * for example `small.jpg 480w, large.jpg 1024w`. The value is split into candidates * following the HTML specification's srcset parsing algorithm, so a comma only - * separates candidates when it ends a URL or its descriptors; a comma inside a URL, - * as produced by image CDN resizers, belongs to the URL. + * separates candidates when it ends a URL or its descriptors. A comma inside a URL, + * as produced by image CDN resizers, belongs to the URL, and one inside parentheses + * in the descriptors belongs to the descriptors. * * Each URL is checked with {@see wp_kses_bad_protocol()}. A candidate whose URL that * check would change is removed in full, descriptors included, because stripping a @@ -3234,19 +3235,8 @@ function wp_kses_sanitize_srcset( string $srcset, array $allowed_protocols ): st $whitespace = " \t\f\r\n"; $length = strlen( $srcset ); $at = 0; - $result = ''; - - /* - * Candidates are only copied once one has to be removed. Until then the - * scan allocates nothing but the URLs it checks, and an all-allowed value - * is returned as the very string that was passed in. - * - * On the first removal the scan restarts to rebuild the output; candidates - * before `$checked_until` passed their check in the first pass and are not - * checked again. - */ - $rebuild = false; - $checked_until = 0; + $kept = array(); + $dropped = false; while ( $at < $length ) { $at += strspn( $srcset, "{$whitespace},", $at ); @@ -3260,35 +3250,33 @@ function wp_kses_sanitize_srcset( string $srcset, array $allowed_protocols ): st $url = rtrim( substr( $srcset, $at, $url_length ), ',' ); $at += $url_length; - // A URL ended by a comma has no descriptors; otherwise they run to the next comma. + /* + * A URL ended by a comma has no descriptors. Otherwise they run to the next + * comma outside parentheses, or to the end when a parenthesis is left open. + */ if ( strlen( $url ) === $url_length ) { - $at += strcspn( $srcset, ',', $at ); - $candidate_length = $at - $candidate_at; - } else { - $candidate_length = strlen( $url ); - } - - if ( $rebuild && $candidate_at < $checked_until ) { - $is_allowed = true; - } else { - $is_allowed = '/' === $url[0] || 0 === strcasecmp( wp_kses_bad_protocol( $url, $allowed_protocols ), $url ); - } + while ( $at < $length ) { + $at += strcspn( $srcset, ',(', $at ); + if ( $at >= $length || ',' === $srcset[ $at ] ) { + break; + } - if ( $rebuild ) { - if ( $is_allowed ) { - $result .= ( '' === $result ? '' : ', ' ) . rtrim( substr( $srcset, $candidate_at, $candidate_length ), $whitespace ); + $closing_at = strpos( $srcset, ')', $at ); + $at = false === $closing_at ? $length : $closing_at + 1; } - continue; + $candidate = substr( $srcset, $candidate_at, $at - $candidate_at ); + } else { + $candidate = $url; } - if ( ! $is_allowed ) { - $rebuild = true; - $checked_until = $candidate_at; - $at = 0; + if ( '/' === $url[0] || 0 === strcasecmp( wp_kses_bad_protocol( $url, $allowed_protocols ), $url ) ) { + $kept[] = rtrim( $candidate, $whitespace ); + } else { + $dropped = true; } } - return $rebuild ? $result : $srcset; + return $dropped ? implode( ', ', $kept ) : $srcset; } /** diff --git a/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php b/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php index 51a0bf804dd80..b5f96def5bed5 100644 --- a/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php +++ b/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php @@ -53,6 +53,8 @@ public function data_allowed_values_are_returned_unchanged(): array { 'CDN resizer URLs with commas' => array( 'https://resizer.example/cdn-cgi/image/format=auto,onerror=redirect,quality=80,width=412,height=275,dpr=1,fit=crop,gravity=0.5x0.5/https://bucket.example/photo.jpg 412w, https://resizer.example/cdn-cgi/image/format=auto,quality=80,width=824,height=550/https://bucket.example/photo.jpg 824w' ), 'CDN resizer URL without descriptor' => array( 'https://cdn.example/image/format=auto,quality=80/img.jpg 400w, https://cdn.example/image/format=auto,quality=80/img-large.jpg' ), 'root-relative URL containing a colon' => array( '/images/a:b.jpg 1x' ), + 'comma inside descriptor parentheses' => array( 'a.jpg 1x(,)z, b.jpg 2x' ), + 'unclosed descriptor parenthesis' => array( 'a.jpg 1x(, javascript:alert(1) 2x' ), ); } @@ -137,6 +139,18 @@ public function data_candidates_with_disallowed_protocols_are_removed(): array { 'HTTPS://example.com/a.jpg 1x, javascript:alert(1) 2x', 'HTTPS://example.com/a.jpg 1x', ), + 'comma inside descriptor parentheses' => array( + 'a.jpg 1x(,)z,javascript:alert(1) 2x', + 'a.jpg 1x(,)z', + ), + 'URL after descriptor parentheses that look scheme-less' => array( + 'a.jpg 1x(,)/?,javascript:alert(1) 2x', + 'a.jpg 1x(,)/?', + ), + 'unclosed parenthesis runs to the end' => array( + 'javascript:alert(1) 1x(, a.jpg 2x', + '', + ), ); } From b04ba88c16ee7e3f227f22fc000c91ccfbc7f728 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Wed, 28 Oct 2026 12:24:07 -0700 Subject: [PATCH 4/4] KSES: Always normalize the output of wp_kses_sanitize_srcset(). Returning the input unchanged when every candidate is allowed let values the HTML parser treats as parse errors, such as `,,,,` or `a.jpg 1x,, b.jpg 2x`, pass through verbatim. Rejoin the kept candidates with `, ` in every case, so empty candidates and surrounding whitespace never reach the output and callers get a single predictable shape. Add examples to the docblock, drop the repeated explanation of why a candidate is removed whole, and add tests for separator normalization, URL syntax characters passing through untouched, and whitespace ending a URL before a scheme can form. See #29807. Claude-Session: https://claude.ai/code/session_018vwYjRp3dvMdHs8fzQbqqN --- src/wp-includes/kses.php | 40 ++++++++++------- .../tests/kses/wpKsesSanitizeSrcset.php | 44 +++++++++++++++---- 2 files changed, 59 insertions(+), 25 deletions(-) diff --git a/src/wp-includes/kses.php b/src/wp-includes/kses.php index e69cec4d3a204..c67d0beda54d8 100644 --- a/src/wp-includes/kses.php +++ b/src/wp-includes/kses.php @@ -3204,24 +3204,35 @@ function wp_kses_bad_protocol_once2( $scheme, $allowed_protocols ) { * Removes image candidates with a disallowed URL protocol from a `srcset` value. * * A `srcset` attribute holds a comma-separated list of image candidate strings, - * each a URL optionally followed by whitespace and width or density descriptors, - * for example `small.jpg 480w, large.jpg 1024w`. The value is split into candidates - * following the HTML specification's srcset parsing algorithm, so a comma only - * separates candidates when it ends a URL or its descriptors. A comma inside a URL, - * as produced by image CDN resizers, belongs to the URL, and one inside parentheses - * in the descriptors belongs to the descriptors. + * each a URL optionally followed by whitespace and width or density descriptors. + * The value is split into candidates following the HTML specification's srcset + * parsing algorithm, so a comma only separates candidates when it ends a URL or + * its descriptors. A comma inside a URL, as produced by image CDN resizers, belongs + * to the URL, and one inside parentheses in the descriptors belongs to the descriptors. * * Each URL is checked with {@see wp_kses_bad_protocol()}. A candidate whose URL that - * check would change is removed in full, descriptors included, because stripping a - * scheme and keeping the rest would rewrite the URL rather than remove it, and - * descriptors left behind would be read as the next candidate's URL. As in - * {@see esc_url()}, a URL starting with `/` cannot carry a scheme and is kept as is. + * check would alter is removed whole, descriptors included. As in {@see esc_url()}, + * a URL starting with `/` cannot carry a scheme and is kept as is. Descriptors are + * not validated. + * + * The remaining candidates are rejoined with `, `, so whitespace around them and + * empty candidates do not survive into the result. + * + * Example: + * + * // A candidate with a disallowed protocol is removed in full. + * 'a.jpg 1x, c.jpg 3x' === wp_kses_sanitize_srcset( 'a.jpg 1x, javascript:alert(1) 2x, c.jpg 3x', $protocols ); + * + * // A comma inside the URL does not end the candidate. + * 'https://cdn.example/w=400,q=80/a.jpg 400w' === wp_kses_sanitize_srcset( 'https://cdn.example/w=400,q=80/a.jpg 400w', $protocols ); + * + * // Separators are normalized. + * 'a.jpg 1x, b.jpg 2x' === wp_kses_sanitize_srcset( " a.jpg 1x ,,\n b.jpg 2x ", $protocols ); * * The value must be decoded attribute text, as returned by * {@see WP_HTML_Tag_Processor::get_attribute()}, not raw HTML containing character * references. The returned value is decoded text too; callers writing it back into - * HTML must escape it. When every candidate is allowed, the input is returned - * unchanged. Otherwise the remaining candidates are rejoined with `, `. + * HTML must escape it. * * @since 7.2.0 * @@ -3236,7 +3247,6 @@ function wp_kses_sanitize_srcset( string $srcset, array $allowed_protocols ): st $length = strlen( $srcset ); $at = 0; $kept = array(); - $dropped = false; while ( $at < $length ) { $at += strspn( $srcset, "{$whitespace},", $at ); @@ -3271,12 +3281,10 @@ function wp_kses_sanitize_srcset( string $srcset, array $allowed_protocols ): st if ( '/' === $url[0] || 0 === strcasecmp( wp_kses_bad_protocol( $url, $allowed_protocols ), $url ) ) { $kept[] = rtrim( $candidate, $whitespace ); - } else { - $dropped = true; } } - return $dropped ? implode( ', ', $kept ) : $srcset; + return implode( ', ', $kept ); } /** diff --git a/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php b/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php index b5f96def5bed5..efd1ddbbf1e0b 100644 --- a/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php +++ b/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php @@ -37,7 +37,6 @@ public function test_allowed_values_are_returned_unchanged( string $srcset ): vo public function data_allowed_values_are_returned_unchanged(): array { return array( 'empty value' => array( '' ), - 'whitespace only' => array( " \t\n" ), 'single URL, no descriptor' => array( 'image.jpg' ), 'relative URLs' => array( 'path/to/test.png 1x, path/to/test-2x.png 2x' ), 'root-relative URLs' => array( '/test.png 1x, /test-2x.png 2x' ), @@ -46,15 +45,46 @@ public function data_allowed_values_are_returned_unchanged(): array { 'uppercase scheme' => array( 'HTTPS://example.com/a.jpg 1x, Http://example.com/b.jpg 2x' ), 'decimal density descriptors' => array( 'image-1x.jpg 1x, image-1.5x.jpg 1.5x, image-2x.jpg 2x' ), 'no descriptor on the last candidate' => array( 'small.jpg 300w, medium.jpg 600w, large.jpg' ), - 'no whitespace after the comma' => array( 'image1.jpg 1x,image2.jpg 2x' ), - 'extra whitespace is preserved' => array( ' image1.jpg 1x , image2.jpg 2x ' ), - 'newlines are preserved' => array( "image1.jpg 1x,\nimage2.jpg 2x" ), + 'tab between URL and descriptor' => array( "image1.jpg\t1x, image2.jpg\t2x" ), + 'URL syntax characters are kept as is' => array( '/image-service?yolo=meme&size=large 2x, /image-service?size=small&q="80" 1x' ), 'commas inside a URL belong to the URL' => array( 'http://localhost/test.png,big 1x, http://localhost/test.png,medium 2x' ), 'CDN resizer URLs with commas' => array( 'https://resizer.example/cdn-cgi/image/format=auto,onerror=redirect,quality=80,width=412,height=275,dpr=1,fit=crop,gravity=0.5x0.5/https://bucket.example/photo.jpg 412w, https://resizer.example/cdn-cgi/image/format=auto,quality=80,width=824,height=550/https://bucket.example/photo.jpg 824w' ), 'CDN resizer URL without descriptor' => array( 'https://cdn.example/image/format=auto,quality=80/img.jpg 400w, https://cdn.example/image/format=auto,quality=80/img-large.jpg' ), 'root-relative URL containing a colon' => array( '/images/a:b.jpg 1x' ), 'comma inside descriptor parentheses' => array( 'a.jpg 1x(,)z, b.jpg 2x' ), 'unclosed descriptor parenthesis' => array( 'a.jpg 1x(, javascript:alert(1) 2x' ), + 'descriptors are not sanitized as URLs' => array( 'a.jpg javascript:alert(1), b.jpg 2x' ), + 'whitespace ends the URL before a scheme forms' => array( "a.jpg 1x, java\tscript:alert(1) 2x" ), + ); + } + + /** + * @ticket 29807 + * + * @dataProvider data_separators_are_normalized + * + * @param string $srcset Value with irregular whitespace or commas between candidates. + * @param string $expected Value with the candidates rejoined with `, `. + */ + public function test_separators_are_normalized( string $srcset, string $expected ): void { + $this->assertSame( $expected, wp_kses_sanitize_srcset( $srcset, $this->allowed_protocols ) ); + } + + /** + * @return array + */ + public function data_separators_are_normalized(): array { + return array( + 'whitespace only' => array( " \t\n", '' ), + 'commas only' => array( ',,,,', '' ), + 'commas and whitespace only' => array( ' , , ', '' ), + 'no whitespace after the comma' => array( 'image1.jpg 1x,image2.jpg 2x', 'image1.jpg 1x, image2.jpg 2x' ), + 'surrounding whitespace' => array( ' image1.jpg 1x , image2.jpg 2x ', 'image1.jpg 1x, image2.jpg 2x' ), + 'newline after the comma' => array( "image1.jpg 1x,\nimage2.jpg 2x", 'image1.jpg 1x, image2.jpg 2x' ), + 'empty candidate between two commas' => array( 'image1.jpg 1x,, image2.jpg 2x', 'image1.jpg 1x, image2.jpg 2x' ), + 'empty candidate with whitespace' => array( 'image1.jpg 1x, , image2.jpg 2x', 'image1.jpg 1x, image2.jpg 2x' ), + 'trailing comma' => array( 'image1.jpg 1x, image2.jpg 2x,', 'image1.jpg 1x, image2.jpg 2x' ), + 'leading comma' => array( ', image1.jpg 1x', 'image1.jpg 1x' ), ); } @@ -119,10 +149,6 @@ public function data_candidates_with_disallowed_protocols_are_removed(): array { 'a.jpg 2q,javascript:alert(1) 2x', 'a.jpg 2q', ), - 'descriptors are not sanitized as URLs' => array( - 'a.jpg javascript:alert(1), b.jpg 2x', - 'a.jpg javascript:alert(1), b.jpg 2x', - ), 'scheme that empties under wp_kses_bad_protocol' => array( 'a:b:c:d:e:f:g 100w, safe.jpg 200w', 'safe.jpg 200w', @@ -131,7 +157,7 @@ public function data_candidates_with_disallowed_protocols_are_removed(): array { 'a:b:c:d:e:f:g, safe.jpg 200w', 'safe.jpg 200w', ), - 'separators are normalized once a candidate is removed' => array( + 'irregular separators around a removed candidate' => array( " a.jpg 1x ,\n\tjavascript:alert(1) 2x ,b.jpg 3x ", 'a.jpg 1x, b.jpg 3x', ),