Incorrect return type in `WP_Rewrite::using_index_permalinks()`.

[costdev]

Overview

In [8899], ​WP_Rewrite::using_index_permalinks() was documented to return bool. However, its actual return type is int|false.

A guard explicitly returns false, however the final return statement returns the result of a preg_match() call.

Per ​the PHP manual:

preg_match() returns 1 if the pattern matches given subject, 0 if it does not, or false on failure.

Docs change, or bool cast?

Docs change

We could change the return type to:

@return int|false 1 if permalink links are enabled and index.php is in the URL,
                  0 if permalinks are enabled but there was a failure,
                  false if permalinks are disabled.

However, I think this is an overly complicated return for a method called using_index_permalinks() that suggests, and is currently documented as, a simple bool return type.

bool cast: Is there a backward compatibility concern?

Yes, but a very small one.

WP Directory shows:

• ​all 135 plugins and ​all 138 themes that call this method use a loose check on the result, treating 0 and false as "No, it does not use index permalinks". These are safe from a (bool) cast.
• It's possible that someone out there is using 1 === $wp_rewrite->using_index_permalinks(), but as the method is documented to return bool, they're most likely using a loose check after an assumed attempt of true === would have always failed, as the method currently never returns true.

When does preg_match() return false?

Per PHP-src, a call to ​preg_match eventually leads to ​pcre_get_compiled_regex_cache_ex.

The following failure conditions are listed:

• ​Empty regular expression
• ​Delimiter must not be alphanumeric, backslash, or NUL
• ​No ending delimiter '%c' found
• ​No ending matching delimiter '%c' found
• ​Unknown modifier '%c'
• ​NUL is not a valid modifier
• ​The /e modifier is no longer supported, use preg_replace_callback instead
• ​Failed to generate locale character tables
• ​Compilation failed: %s at offset %zu
• ​Internal pcre2_pattern_info() error %d
• ​Internal pcre_pattern_info() error %d

Which of these failures could happen here?

In the context of WP_Rewrite::using_index_permalinks(), there is only one possible failure: WP_Rewrite::$index contains the delimiter, #. ​3v4l.

The index property is set to 'index.php' by default. However, it's a public property and could therefore be directly overridden, or set in a subclass of WP_Rewrite.

WP Directory shows:

• ​0 plugins directly assign a value to this property.
• ​0 themes directly assign a value to this property.
• ​1 plugin extends the WP_Rewrite class, but does not set a value for the property.
• ​0 themes extend the WP_Rewrite class.

Can we prevent the potential failure?

Yes, by using preg_quote( $this->index, '#' ). ​3v4l.

Should we prevent the potential failure?

As demonstrated by the failure, the regex is currently susceptible to special characters. When the default value of $this->index ('index.php') is concatenated, the resulting pattern is '#^/*index.php#'.

See the .? The preg_match() call is currently vulnerable to index[any character except newline]php. ​3v4l.

Given this, it's possible that a developer out there has set a custom $index value that contains a regex pattern. However, I think that's very unlikely.

Contrarily, the flawed regex has the potential to cause false positives if, for example, the permalink structure starts with /index2php. While that's also unlikely, using preg_quote() here would prevent that false positive.

---

Call for opinions

1. Should we just change the documented return type and description? If so, what makes sense here?
2. OR should we cast the result to bool?
3. ADDITIONALLY should we use preg_quote() here to prevent the potential failure?