Opened 9 years ago
Last modified 3 months ago
#40566 new enhancement
add_query_arg() returns only URL fragments in certain circumstances
| Reported by: | DavidAnderson | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Future Release |
| Component: | General | Version: | |
| Severity: | normal | Keywords: | has-patch |
| Cc: | Focuses: | docs |
Description (last modified by )
The documentation for add_query_arg() - https://developer.wordpress.org/reference/functions/add_query_arg/ - and its docblock, both claim that it returns a URL. And this appears to be how it is commonly used. (I started looking into this because of it being used in WooCommerce in this way, a wrong assumption which ulimately causes a reproducible-every-time 404 on my webserver on password reset requests via WooCommerce).
e.g. in the documentation: "Retrieves a modified URL query string." "You can rebuild the URL and append query variables to the URL query by using this function". It is also recommended to use esc_url() on what is returned - a function that states that it acts upon URLs, implying that what is returned is indeed a URL.
The docblock says "Retrieves a modified URL query string" - which isn't a very clear statement (does it return a query string? Or a URL *with* modified query string?). It continues later with "Omitting the URL from either use results in the current URL being used (the value of $_SERVER['REQUEST_URI'])".
The last clause is where the problem comes in. $_SERVER['REQUEST_URI'], actually stores a path that is relative to the current host, only (i.e. not "U"niversal). Another beautiful bit of PHP mis-design: http://php.net/manual/en/reserved.variables.server.php - "The URI which was given in order to access this page; for instance, '/index.html'."
As a result, despite the documentation and expectation, add_query_arg() returns relative fragments. And when these are passed to wp_redirect(), as WooCommerce does, the result can be, as in my case, that the webserver gives a 404. (This appears to be related to changes to relative path handling in CGI output in recent versions of lighttpd. The problem is CGI-specific I think - it relates to how a webserver handles the output of a CGI script when that output has a Location: header without a full URL in it).
It appears to me that in the default case of no URL being specified, add_query_arg() should return an actual URL, instead of a relative path.
Perhaps related: #14062
Change History (5)
#2
@
9 years ago
- Description modified (diff)
- Summary Defect: add_query_arg() returns only URL fragments in certain circumstances → add_query_arg() returns only URL fragments in certain circumstances
- Version trunk
This ticket was mentioned in Slack in #core by noisysocks. View the logs.
6 years ago
#4
@
6 years ago
- Focuses docs added
- Keywords needs-patch added
- Milestone Awaiting Review → Future Release
- Type defect (bug) → enhancement
Sounds like all that is required here is to update the docs for add_query_arg() (and possibly remove_query_arg()) to warn that if $_SERVER['REQUEST_URI'] is a relative path then the returned value will be as well.
This ticket was mentioned in PR #12347 on WordPress/wordpress-develop by @shreya0shrivastava.
3 months ago
#5
- Keywords has-patch added; needs-patch removed
Updates the docblock for add_query_arg() to clarify that omitting the base URL returns a root-relative path, not an absolute URL, and that home_url() should be used when an absolute URL is required.
Trac ticket: https://core.trac.wordpress.org/ticket/40566
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
N.B. I believe, on reading the CGI spec, that lighttpd has a bug here. That's just the back-story - I think there's still an issue with add_query_arg() indicating that it returns an absolute location in the default case of no URL being supplied as input, rather than a relative one.