[wp-trac] [WordPress Trac] #40566: add_query_arg() returns only URL fragments in certain circumstances (was: Defect: add_query_arg() returns only URL fragments in certain circumstances)

[WordPress Trac]

#40566: add_query_arg() returns only URL fragments in certain circumstances
---------------------------+------------------------------
 Reporter:  DavidAnderson  |       Owner:
     Type:  defect (bug)   |      Status:  new
 Priority:  normal         |   Milestone:  Awaiting Review
Component:  General        |     Version:
 Severity:  normal         |  Resolution:
 Keywords:                 |     Focuses:
---------------------------+------------------------------
Changes (by ocean90):

 * version:  trunk =>


Old description:

> 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: https://core.trac.wordpress.org/ticket/14062

New description:

 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

--

--
Ticket URL: <https://core.trac.wordpress.org/ticket/40566#comment:2>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform