Empty endpoint same as missing endpoint: empty vs isset query_var.

[5ubliminal]

​http://wordpress.org/support/topic/custom-rewrite-approach-with-add_rewrite_endpoint

If I have a /test/ endpoint, it will not show in query_vars unless it has a value. So /test/1 will have query_vars[test]=1 but /test/ will NOT have query_vars[test]=null in query_vars. Empty endpoints should be present with null value in query_vars so they can be tested with isset() for value-less existence.

Hope it make sense. Tested in 3.0.1 and this problem exists.

[dd32]

See also #10710 which may have fixed this already in trunk(3.1)

[5ubliminal]

That's it! Thanks.

[johnbillion]

This wasn't in fact fixed in #10710. An endpoint without a value is still indistinguishable from no endpoint. All #10710 done was to explicitly set all query vars to an empty string.

With a test endpoint, example.com/test/ and example.com/ both give us query_vars[test]='' (an empty string), making them indistinguishable.

Additionally, the inline doc for add_rewrite_endpoint() is misleading as it states:

Add an endpoint, like /trackback/.

which suggests endpoints without values are possible.

[johnbillion]

Here's my take on this. To allow an empty endpoint (ie. an endpoint with no trailing value) we need to set the endpoint's query var to something suitable when the endpoint is present but empty.

In the attached patch I've opted to use string 'true' for empty endpoints. add_rewrite_endpoint() gets an optional third parameter, $allow_empty, which tells WP_Rerite to set the endpoint query var to string 'true' when it's empty. The addition of this parameter maintains backwards compatibilty with anything that's using add_rewrite_endpoint() and not expecting an empty endpoint to have a value.

Inline docs have also been updated.

Feedback welcome.

[nacin]

otto42 and I recently conducted a thought experiment on all of this as well. #10710 did actually make this much easier. Previously, the value didn't exist in WP_Query->query_vars at all. Now it does, it's just empty. You can check for isset() against it and it works just fine.

The problem is with WP_Query::get(), which returns '' instead of false when nothing is set.

I'd be willing to modify that to be false. Because it's a query variable, it normally will always return a string, so I doubt the change will hurt any existing code.

The other option is a WP_Query::exists() method, which simply returns the results of isset(). Unfortunately this would not work easily for core query variables, as those are set to empty (but not null) values in fill_query_vars(). At that point we'd probably want to set all string-based QVs to null instead, and then keep a list of array-based QVs to then return the results of empty() on, or something.

I don't think another rewrite rule is necessary. Additionally, it'd end up with 'true' as a string, rather than a boolean. And a string of '1' might conflict with situations where you do have the ability to specify a endpoint values in addition to leaving it blank. Because this was actually fixed in some regard, I'm going to close it again. Feel free to open a new ticket to address the ::get() issue, but at the very least, you now have a workaround.

[johnbillion]

Interesting points nacin. The problem is indeed with WP_Query->get() (and get_query_var()).

The return value of string true in my patch was intentional. It removed the ambiguity with values of a non-empty endpoint (eg. if '1' was used instead of 'true'), and would be consistent with the value of ?my_endpoint=true if rewrites weren't enabled. A moot point though.

+1 for changing WP_Query->get() so it returns boolean false for unset query vars, and/or introducing an exists() method.

[nacin]

#19243

[nacin]

#16471