Skip to content

fix: update the logic of fetching current url in loginout block#70031

Merged
Mamaduka merged 3 commits intoWordPress:trunkfrom
SH4LIN:fix/loginout-block-redirection-url-fetching
May 1, 2025
Merged

fix: update the logic of fetching current url in loginout block#70031
Mamaduka merged 3 commits intoWordPress:trunkfrom
SH4LIN:fix/loginout-block-redirection-url-fetching

Conversation

@SH4LIN
Copy link
Copy Markdown
Contributor

@SH4LIN SH4LIN commented Apr 30, 2025

What?

Closes: #70024

Why?

Our current method for retrieving the current URL is as follows:

$current_url = ( is_ssl() ? 'https://' : 'http://' ) . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'];

This approach relies on is_ssl() and $_SERVER['HTTP_HOST'], and it accesses $_SERVER['HTTP_HOST'] without checking if it is set. It also lacks proper usage of wp_unslash() and sanitization.

What is your proposed solution?

Why rely on $_SERVER['HTTP_HOST'] and is_ssl() when we can construct the URL directly using:

home_url( wp_unslash( sanitize_url( $_SERVER['REQUEST_URI'] ) ) )

This provides a more secure and WordPress-native approach.

How?

  • We are checking whether REQUEST_URI is set or not if it is not set then use home_url() if it is set then use home_url() with REQUEST_URI as it's path

Testing Instructions

  1. Add loginout block in your page or post.
  2. After login or logout it should redirect to the page from where the request initiated

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 30, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: SH4LIN <sh4lin@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@Mamaduka Mamaduka added [Type] Enhancement A suggestion for improvement. [Block] Login/out Affects the Login/out Block [Status] In discussion Used to indicate that an issue is in the process of being discussed labels May 1, 2025
Copy link
Copy Markdown
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Could you update each section of the description according to the actual changes?

Comment on lines 19 to 21
// This current url fetching logic matches with the core: https://github.com/WordPress/WordPress/blob/6612d90f6c8ee9e917dc2dfcbcc24e120a5746ea/wp-includes/general-template.php#L528
// Build the redirect URL.
$current_url = ( is_ssl() ? 'https://' : 'http://' ) . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// This current url fetching logic matches with the core: https://github.com/WordPress/WordPress/blob/6612d90f6c8ee9e917dc2dfcbcc24e120a5746ea/wp-includes/general-template.php#L528
// Build the redirect URL.
$current_url = ( is_ssl() ? 'https://' : 'http://' ) . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'];
/*
* Build the redirect URL. This current url fetching logic matches with the core.
*
* @see https://github.com/WordPress/wordpress-develop/blob/6bf62e58d21739938f3bb3f9e16ba702baf9c2cc/src/wp-includes/general-template.php#L528.
*/
$current_url = ( is_ssl() ? 'https://' : 'http://' ) . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'];
  • Let's use the PHPDoc format.
  • It would be good to reference the development repo (WordPress/wordpress-develop), not the mirror repo (WordPress/WordPress),

@SH4LIN SH4LIN requested a review from t-hamano May 1, 2025 08:00
Copy link
Copy Markdown
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@Mamaduka Mamaduka merged commit 72c922b into WordPress:trunk May 1, 2025
60 checks passed
@github-actions github-actions bot added this to the Gutenberg 20.8 milestone May 1, 2025
chriszarate pushed a commit to chriszarate/gutenberg that referenced this pull request Jul 1, 2025
Co-authored-by: SH4LIN <sh4lin@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Block] Login/out Affects the Login/out Block [Status] In discussion Used to indicate that an issue is in the process of being discussed [Type] Enhancement A suggestion for improvement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhancement: core/loginout block

3 participants