WordPress Core strips $_GET['error'] occasionally

[javorszky]

I have a plugin that is an OAuth2 consumer for integrating with Stripe Connect.

I created a new custom endpoint by adding a query var, and a rewrite rule, so everything that lands on /stripe_connect will get dealt with by my plugin's code.

If user denies the connection request at Stripe, they are redirected back to my site with roughly the following URL params in tow:

/stripe_connect?state=3__5e4e4d4c9df8e6948a33fdfb44f75c0f&error=access_denied&error_description=The+user+denied+your+request

• state is a custom param I set that gets replayed to me
• error is access_denied, which is the standard that Stripe will do in this case, see ​https://stripe.com/docs/connect/oauth-reference#get-authorize-errors
• error_description is a human readable problem

However in parse_request, a variable by the name of $error gets set to 404 at the beginning, and as it matches the rules, if it's still 404 (ie no other error popped up, it will then unset $_GET['error'].

Link to code: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/class-wp.php#L260

Which is something I'd actually need to deal with.

Currently the way to get around it is to use $_REQUEST instead of $_GET, however $_REQUEST also has POST variables in it, so I can't make sure that the error I'm getting is actually due to a query param.

I also haven't found a ticket that had this listed as a problem.

What was the reasoning for unsetting that $_GET var?

I see that they were added originally in [1570] (14 years ago), however is that still a valid reason?

[SergeyBiryukov]

What was the reasoning for unsetting that $_GET var?

error is one of the public query vars, and the further code checks if any of those vars is set via $_POST or $_GET​, so it seems the reason is to make sure $this->query_vars['error'] is not set when there's no error.

There might be a better way to do that though, something like 44965.patch​.

[pento]

@SergeyBiryukov: Why the unset( $this->query_vars['error'] )? It seems like the original purpose ([1570]) was that WP_Query would generate a 404 if the error query var was not an empty string. WP_Query now only generates a 404 if the error query var is set to '404'.

[gabrielchung1128]

When I am constructing a test case for this issue, I have found that when the test case is run, the global scope is removed. It means that $_GET will be cleared. It makes it difficult to create a test case for it. Should we have a configuration variable to bypass cleaning the global scope? Thanks.

Here is the line of code of removing the global scope:
​https://github.com/WordPress/wordpress-develop/blob/c62eab00a79d76ef94f01ee841cfa7708ad9811c/tests/phpunit/includes/abstract-testcase.php#L129

[gabrielchung1128]

Test Case

[gabrielchung1128]

I have figured a way to set and store the $_GET variables.

Thanks.

[pentatonicfunk]

noticed the same issue while using WP-REST-API, error param is gone from WP_REST_Request::get_param() and WP_REST_Request::get_params()