Make WordPress Core

Opened 6 years ago

Last modified 20 months ago

#44965 accepted defect (bug)

WordPress Core strips $_GET['error'] occasionally

Reported by: javorszky's profile javorszky Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Query Keywords: has-patch has-unit-tests dev-feedback
Focuses: Cc:

Description (last modified by SergeyBiryukov)

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:


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:

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?

Attachments (2)

44965.patch (1004 bytes) - added by SergeyBiryukov 6 years ago.
44965-2.patch (2.2 KB) - added by gabrielchung1128 5 years ago.
Test Case

Download all attachments as: .zip

Change History (14)

#1 @SergeyBiryukov
6 years ago

  • Component changed from General to Query
  • Description modified (diff)
  • Keywords has-patch needs-unit-tests added

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.

Last edited 6 years ago by SergeyBiryukov (previous) (diff)

#2 @SergeyBiryukov
6 years ago

  • Milestone changed from Awaiting Review to 4.9.9
  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

#3 @pento
6 years ago

  • Milestone changed from 4.9.9 to 5.0.1

This ticket was mentioned in Slack in #core-php by sergey. View the logs.

6 years ago

#5 @pento
5 years ago

  • Milestone changed from 5.0.1 to 5.0.2

#6 @SergeyBiryukov
5 years ago

  • Milestone changed from 5.0.2 to 5.1

#7 @pento
5 years ago

@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'.

#8 @pento
5 years ago

  • Milestone changed from 5.1 to 5.2

#9 @gabrielchung1128
5 years ago

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:

5 years ago

Test Case

#10 @gabrielchung1128
5 years ago

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


#11 @pento
5 years ago

  • Keywords has-unit-tests dev-feedback added; needs-unit-tests removed
  • Milestone changed from 5.2 to Future Release

#12 @pentatonicfunk
20 months ago

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()

Note: See TracTickets for help on using tickets.