From f1f750c7fafb8d3df748ecee9ba3f0ad66276120 Mon Sep 17 00:00:00 2001 From: Antony Agrios Date: Fri, 9 Dec 2022 17:12:14 +0200 Subject: [PATCH 1/8] fix: issue with already encoded urls --- .../src/navigation-link/index.php | 2 +- ...ass-block-library-navigation-link-test.php | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation-link/index.php b/packages/block-library/src/navigation-link/index.php index f32437b41a3f84..58e822e27fa9c5 100644 --- a/packages/block-library/src/navigation-link/index.php +++ b/packages/block-library/src/navigation-link/index.php @@ -171,7 +171,7 @@ function render_block_core_navigation_link( $attributes, $content, $block ) { // Start appending HTML attributes to anchor tag. if ( isset( $attributes['url'] ) ) { - $html .= ' href="' . esc_url( $attributes['url'] ) . '"'; + $html .= ' href="' . esc_url( urldecode( $attributes['url'] ) ) . '"'; } if ( $is_active ) { diff --git a/phpunit/class-block-library-navigation-link-test.php b/phpunit/class-block-library-navigation-link-test.php index bb6cc33fb2e8ba..6a0ba8375f74be 100644 --- a/phpunit/class-block-library-navigation-link-test.php +++ b/phpunit/class-block-library-navigation-link-test.php @@ -212,6 +212,35 @@ public function test_returns_link_for_plain_link() { ); } + public function test_returns_link_for_decoded_link() { + $parsed_blocks = parse_blocks( + '' + ); + $this->assertEquals( 1, count( $parsed_blocks ) ); + + $navigation_link_block = new WP_Block( $parsed_blocks[0], array() ); + + echo( + render_block_core_navigation_link( + $navigation_link_block->attributes, + array(), + $navigation_link_block + ) + ); + + $this->assertEquals( + true, + strpos( + render_block_core_navigation_link( + $navigation_link_block->attributes, + array(), + $navigation_link_block + ), + 'https://example.com?data=lzB%2Fzd%2FZA%3D%3D' + ) !== false + ); + } + public function test_returns_empty_when_custom_post_type_draft() { $page_id = self::$custom_draft->ID; From 8256ad3354fa145636541ba15e88a2185aac3a5d Mon Sep 17 00:00:00 2001 From: Antony Agrios Date: Fri, 9 Dec 2022 19:26:22 +0200 Subject: [PATCH 2/8] fix: test --- phpunit/class-block-library-navigation-link-test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpunit/class-block-library-navigation-link-test.php b/phpunit/class-block-library-navigation-link-test.php index 6a0ba8375f74be..622ed6324e6581 100644 --- a/phpunit/class-block-library-navigation-link-test.php +++ b/phpunit/class-block-library-navigation-link-test.php @@ -236,7 +236,7 @@ public function test_returns_link_for_decoded_link() { array(), $navigation_link_block ), - 'https://example.com?data=lzB%2Fzd%2FZA%3D%3D' + 'https://example.com/?data=lzB%2Fzd%2FZA%3D%3D' ) !== false ); } From 4ae395e0e23e2f2b3349de77a1ca289edad62c72 Mon Sep 17 00:00:00 2001 From: Antony Agrios Date: Tue, 20 Dec 2022 10:01:43 +0200 Subject: [PATCH 3/8] fix: test --- phpunit/class-block-library-navigation-link-test.php | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/phpunit/class-block-library-navigation-link-test.php b/phpunit/class-block-library-navigation-link-test.php index 622ed6324e6581..4cbd01e1408911 100644 --- a/phpunit/class-block-library-navigation-link-test.php +++ b/phpunit/class-block-library-navigation-link-test.php @@ -220,18 +220,10 @@ public function test_returns_link_for_decoded_link() { $navigation_link_block = new WP_Block( $parsed_blocks[0], array() ); - echo( - render_block_core_navigation_link( - $navigation_link_block->attributes, - array(), - $navigation_link_block - ) - ); - $this->assertEquals( true, strpos( - render_block_core_navigation_link( + gutenberg_render_block_core_navigation_link( $navigation_link_block->attributes, array(), $navigation_link_block From 4704426ba929304490529b7a5880d1442a704898 Mon Sep 17 00:00:00 2001 From: Antony Agrios Date: Wed, 21 Dec 2022 12:44:09 +0200 Subject: [PATCH 4/8] fix: Always encode a url before saving --- packages/block-library/src/navigation-link/edit.js | 6 +++++- .../block-library/src/navigation-link/update-attributes.js | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index b5ba89d604abda..4121c0f31af966 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -489,7 +489,11 @@ export default function NavigationLinkEdit( { { - setAttributes( { url: urlValue } ); + updateAttributes( + { url: urlValue }, + setAttributes, + attributes + ); } } label={ __( 'URL' ) } autoComplete="off" diff --git a/packages/block-library/src/navigation-link/update-attributes.js b/packages/block-library/src/navigation-link/update-attributes.js index 4b7ee45ac99111..f618b5f03ef2fe 100644 --- a/packages/block-library/src/navigation-link/update-attributes.js +++ b/packages/block-library/src/navigation-link/update-attributes.js @@ -89,7 +89,7 @@ export const updateAttributes = ( setAttributes( { // Passed `url` may already be encoded. To prevent double encoding, decodeURI is executed to revert to the original string. - ...( newUrl && { url: encodeURI( safeDecodeURI( newUrl ) ) } ), + ...{ url: newUrl ? encodeURI( safeDecodeURI( newUrl ) ) : newUrl }, ...( label && { label } ), ...( undefined !== opensInNewTab && { opensInNewTab } ), ...( id && Number.isInteger( id ) && { id } ), From 3a29c0e0e23ef35f6cce045b52229c7c579a0563 Mon Sep 17 00:00:00 2001 From: Antony Agrios Date: Fri, 23 Dec 2022 18:31:37 +0200 Subject: [PATCH 5/8] Try to decode only urls that are encoded --- .../src/navigation-link/index.php | 36 ++++++++++++++- ...ass-block-library-navigation-link-test.php | 46 ++++++++++++------- 2 files changed, 64 insertions(+), 18 deletions(-) diff --git a/packages/block-library/src/navigation-link/index.php b/packages/block-library/src/navigation-link/index.php index 868ed2221bfbb3..3a1743334c2cd1 100644 --- a/packages/block-library/src/navigation-link/index.php +++ b/packages/block-library/src/navigation-link/index.php @@ -120,6 +120,40 @@ function block_core_navigation_link_render_submenu_icon() { return ''; } +/** + * Checks if the given url is encoded + * + * @param string $url The url to check. + * + * @return boolean Whether or not a url is encoded. + */ +function is_url_encoded($url) { + $query = parse_url($url, PHP_URL_QUERY); + $query_params = wp_parse_args($query); + foreach($query_params as $query_param){ + if(rawurldecode($query_param) !== $query_param){ + return true; + } + } + return false; +} + + +/** + * Decodes a url if it's encoded, returning the same url if not. + * + * @param string $url The url to decode. + * + * @return string $url Returns the decoded url. + */ +function urldecode_once($url){ + if(is_url_encoded($url)){ + return rawurldecode($url); + } + return $url; +} + + /** * Renders the `core/navigation-link` block. * @@ -171,7 +205,7 @@ function render_block_core_navigation_link( $attributes, $content, $block ) { // Start appending HTML attributes to anchor tag. if ( isset( $attributes['url'] ) ) { - $html .= ' href="' . esc_url( urldecode( $attributes['url'] ) ) . '"'; + $html .= ' href="' . esc_url( urldecode_once( $attributes['url'] ) ) . '"'; } if ( $is_active ) { diff --git a/phpunit/class-block-library-navigation-link-test.php b/phpunit/class-block-library-navigation-link-test.php index 4cbd01e1408911..2e2f232206046b 100644 --- a/phpunit/class-block-library-navigation-link-test.php +++ b/phpunit/class-block-library-navigation-link-test.php @@ -213,24 +213,36 @@ public function test_returns_link_for_plain_link() { } public function test_returns_link_for_decoded_link() { - $parsed_blocks = parse_blocks( - '' - ); - $this->assertEquals( 1, count( $parsed_blocks ) ); - $navigation_link_block = new WP_Block( $parsed_blocks[0], array() ); - - $this->assertEquals( - true, - strpos( - gutenberg_render_block_core_navigation_link( - $navigation_link_block->attributes, - array(), - $navigation_link_block - ), - 'https://example.com/?data=lzB%2Fzd%2FZA%3D%3D' - ) !== false - ); + $urls_before_render = [ + "https://example.com/?id=10&data=lzB%252Fzd%252FZA%253D%253D", + "https://example.com/?id=10&data=lzB%2Fzd%FZA%3D%3D", + "https://example.com/?id=10&data=1234", + ]; + + $urls_after_render = [ + 'https://example.com/?id=10&data=lzB%2Fzd%2FZA%3D%3D', + "https://example.com/?id=10&data=lzB%2Fzd%FZA%3D%3D", + 'https://example.com/?id=10&data=1234', + ]; + + foreach ( $urls_before_render as $idx => $link ) { + $parsed_blocks = parse_blocks(''); + $this->assertEquals( 1, count( $parsed_blocks ) ); + $block = $parsed_blocks[0]; + $navigation_link_block = new WP_Block( $block, array() ); + $this->assertEquals( + true, + strpos( + gutenberg_render_block_core_navigation_link( + $navigation_link_block->attributes, + array(), + $navigation_link_block + ), + $urls_after_render[$idx] + ) !== false + ); + }; } public function test_returns_empty_when_custom_post_type_draft() { From 94307546f54464b59868676f53691e8c2f9a9d2b Mon Sep 17 00:00:00 2001 From: Antony Agrios Date: Tue, 3 Jan 2023 16:52:35 +0200 Subject: [PATCH 6/8] change the naming to match WordPress core naming pattern --- packages/block-library/src/navigation-link/index.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/navigation-link/index.php b/packages/block-library/src/navigation-link/index.php index 3a1743334c2cd1..1fad7c8e65f077 100644 --- a/packages/block-library/src/navigation-link/index.php +++ b/packages/block-library/src/navigation-link/index.php @@ -146,7 +146,7 @@ function is_url_encoded($url) { * * @return string $url Returns the decoded url. */ -function urldecode_once($url){ +function maybe_urldecode($url){ if(is_url_encoded($url)){ return rawurldecode($url); } @@ -205,7 +205,7 @@ function render_block_core_navigation_link( $attributes, $content, $block ) { // Start appending HTML attributes to anchor tag. if ( isset( $attributes['url'] ) ) { - $html .= ' href="' . esc_url( urldecode_once( $attributes['url'] ) ) . '"'; + $html .= ' href="' . esc_url( maybe_urldecode( $attributes['url'] ) ) . '"'; } if ( $is_active ) { From 4cfabbbce15a2b37c55e85cfe501a89bf3a9e533 Mon Sep 17 00:00:00 2001 From: Antony Agrios Date: Tue, 3 Jan 2023 17:09:17 +0200 Subject: [PATCH 7/8] change the naming to match WordPress core naming pattern --- .../src/navigation-link/index.php | 43 ++++++++----------- 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/packages/block-library/src/navigation-link/index.php b/packages/block-library/src/navigation-link/index.php index 1fad7c8e65f077..3e46da676710be 100644 --- a/packages/block-library/src/navigation-link/index.php +++ b/packages/block-library/src/navigation-link/index.php @@ -120,25 +120,6 @@ function block_core_navigation_link_render_submenu_icon() { return ''; } -/** - * Checks if the given url is encoded - * - * @param string $url The url to check. - * - * @return boolean Whether or not a url is encoded. - */ -function is_url_encoded($url) { - $query = parse_url($url, PHP_URL_QUERY); - $query_params = wp_parse_args($query); - foreach($query_params as $query_param){ - if(rawurldecode($query_param) !== $query_param){ - return true; - } - } - return false; -} - - /** * Decodes a url if it's encoded, returning the same url if not. * @@ -146,11 +127,23 @@ function is_url_encoded($url) { * * @return string $url Returns the decoded url. */ -function maybe_urldecode($url){ - if(is_url_encoded($url)){ - return rawurldecode($url); - } - return $url; +function block_core_navigation_link_maybe_urldecode($url){ + $is_url_encoded = false; + $query = parse_url($url, PHP_URL_QUERY); + $query_params = wp_parse_args($query); + + foreach($query_params as $query_param){ + if(rawurldecode($query_param) !== $query_param){ + $is_url_encoded = true; + break; + } + } + + if($is_url_encoded){ + return rawurldecode($url); + } + + return $url; } @@ -205,7 +198,7 @@ function render_block_core_navigation_link( $attributes, $content, $block ) { // Start appending HTML attributes to anchor tag. if ( isset( $attributes['url'] ) ) { - $html .= ' href="' . esc_url( maybe_urldecode( $attributes['url'] ) ) . '"'; + $html .= ' href="' . esc_url( block_core_navigation_link_maybe_urldecode( $attributes['url'] ) ) . '"'; } if ( $is_active ) { From 2cb90f0176705332faf8da666edb8ab870d6d199 Mon Sep 17 00:00:00 2001 From: Antony Agrios Date: Wed, 4 Jan 2023 13:57:11 +0200 Subject: [PATCH 8/8] fix: Fix lint issues --- .../src/navigation-link/index.php | 14 ++++---- ...ass-block-library-navigation-link-test.php | 32 +++++++++---------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/packages/block-library/src/navigation-link/index.php b/packages/block-library/src/navigation-link/index.php index 3e46da676710be..0eab52dc6ae227 100644 --- a/packages/block-library/src/navigation-link/index.php +++ b/packages/block-library/src/navigation-link/index.php @@ -127,20 +127,20 @@ function block_core_navigation_link_render_submenu_icon() { * * @return string $url Returns the decoded url. */ -function block_core_navigation_link_maybe_urldecode($url){ +function block_core_navigation_link_maybe_urldecode( $url ) { $is_url_encoded = false; - $query = parse_url($url, PHP_URL_QUERY); - $query_params = wp_parse_args($query); + $query = parse_url( $url, PHP_URL_QUERY ); + $query_params = wp_parse_args( $query ); - foreach($query_params as $query_param){ - if(rawurldecode($query_param) !== $query_param){ + foreach ( $query_params as $query_param ) { + if ( rawurldecode( $query_param ) !== $query_param ) { $is_url_encoded = true; break; } } - if($is_url_encoded){ - return rawurldecode($url); + if ( $is_url_encoded ) { + return rawurldecode( $url ); } return $url; diff --git a/phpunit/class-block-library-navigation-link-test.php b/phpunit/class-block-library-navigation-link-test.php index a2330b90106f20..0205e546d7bfac 100644 --- a/phpunit/class-block-library-navigation-link-test.php +++ b/phpunit/class-block-library-navigation-link-test.php @@ -214,23 +214,23 @@ public function test_returns_link_for_plain_link() { public function test_returns_link_for_decoded_link() { - $urls_before_render = [ - "https://example.com/?id=10&data=lzB%252Fzd%252FZA%253D%253D", - "https://example.com/?id=10&data=lzB%2Fzd%FZA%3D%3D", - "https://example.com/?id=10&data=1234", - ]; - - $urls_after_render = [ - 'https://example.com/?id=10&data=lzB%2Fzd%2FZA%3D%3D', - "https://example.com/?id=10&data=lzB%2Fzd%FZA%3D%3D", - 'https://example.com/?id=10&data=1234', - ]; + $urls_before_render = array( + 'https://example.com/?id=10&data=lzB%252Fzd%252FZA%253D%253D', + 'https://example.com/?id=10&data=lzB%2Fzd%FZA%3D%3D', + 'https://example.com/?id=10&data=1234', + ); + + $urls_after_render = array( + 'https://example.com/?id=10&data=lzB%2Fzd%2FZA%3D%3D', + 'https://example.com/?id=10&data=lzB%2Fzd%FZA%3D%3D', + 'https://example.com/?id=10&data=1234', + ); foreach ( $urls_before_render as $idx => $link ) { - $parsed_blocks = parse_blocks(''); - $this->assertEquals( 1, count( $parsed_blocks ) ); - $block = $parsed_blocks[0]; - $navigation_link_block = new WP_Block( $block, array() ); + $parsed_blocks = parse_blocks( '' ); + $this->assertEquals( 1, count( $parsed_blocks ) ); + $block = $parsed_blocks[0]; + $navigation_link_block = new WP_Block( $block, array() ); $this->assertEquals( true, strpos( @@ -239,7 +239,7 @@ public function test_returns_link_for_decoded_link() { array(), $navigation_link_block ), - $urls_after_render[$idx] + $urls_after_render[ $idx ] ) !== false ); };