#56160 closed enhancement (duplicate)
Deprecate wp_sanitize_redirect
| Reported by: | malthert | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | Security | Version: | 2.3 |
| Severity: | normal | Keywords: | |
| Cc: | Focuses: |
Description
All places that currently use it are better served with esc_url_raw and there seems to be no correct usage of it anywhere (most plugins use it where esc_url_raw should be used instead).
I'm happy to provide a PR, I just need to get some heads up about how the deprecation process works in WP exactly.
Change History (10)
#2
@
4 years ago
WP core uses it only in 3 functions:
- wp_redirect
- wp_safe_redirect
- wp_validate_redirect
https://developer.wordpress.org/reference/functions/esc_url_raw/
States:
The resulting URL is safe to use in database queries, redirects and HTTP requests.
So if esc_url_raw is safe to use in (= sanitize), then there is no need for wp_sanitize_redirect, as esc_url_raw already does this.
If esc_url_raw is NOT safe for sanitizing data in redirects, this needs to be updated in docs to highlight wp_sanitize_redirect has to be used.
Additionally, it's unclear what the difference between the use cases for sanitize_url and wp_sanitize_redirect are?
This ticket was mentioned in PR #6024 on WordPress/wordpress-develop by @kkmuffme.
3 years ago
#3
- Keywords has-patch added
sanitize_url() description says:
Sanitizes a URL for database or redirect usage.
Therefore a separate function does not make sense, as it creates inconsistent behavior.
#4
@
3 years ago
I want to take over this ticket and created a PR to alias the functions like what happened to esc_url_raw with sanitize_url in WP 6.1. I didn't mark it as deprecated though to keep the external (plugins,...) minimal.
I didn't look at the internals - if these functions actually did the same. In case they did not, the docs for sanitize_url and esc_url_raw should be updated instead to clarify that they're NOT safe for redirects.
This ticket was mentioned in Slack in #core by kkmuffme. View the logs.
3 years ago
#6
@
3 years ago
- Keywords needs-refresh added
The current PR breaks existing tests for wp_sanitize_redirect and wp_validate_redirect so clearly the behavior is not identical.
#7
@
3 years ago
Yes, so the PR should be to update the docs to remove "redirect" from sanitize_url docs?
#8
@
3 years ago
And maybe mark the wp_sanitize_redirect @internal since it doesn't really have a use case outside of wp_(safe_)redirect, does it?
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Hi @malthert, thanks for opening this ticket. I've left a few thoughts below.
[53455] changed
esc_url_raw()calls tosanitize_url()as this is now the recommended function for sanitizing a URL as of WordPress 6.1.For discussion, posterity, and for making committers' lives easier should this get support, could you provide your thoughts on why
esc_url_raw()serves some of these usages better, or where it's more appropriate thanwp_sanitize_redirect()?An impact analysis is an important part of any deprecation. I'll kick it off with some numbers:
sanitize_url_redirect\(.sanitize_url_redirect\(.I've added the
2nd-opinionkeyword to draw attention to this ticket.