Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Undefined index" warning in vip-caching.php #976

Closed
gmazzap opened this issue Oct 1, 2018 · 1 comment · Fixed by #1381
Closed

"Undefined index" warning in vip-caching.php #976

gmazzap opened this issue Oct 1, 2018 · 1 comment · Fixed by #1381

Comments

@gmazzap
Copy link
Contributor

gmazzap commented Oct 1, 2018

The function wpcom_vip_maybe_skip_old_slug_redirect (https://github.com/Automattic/vip-go-mu-plugins/blob/master/vip-helpers/vip-caching.php#L712-L720) in our local environment produces a warning: Undefined index: DOCUMENT_URI.

In our systems, the server variable DOCUMENT_URI is not defined, we have the same value in REQUEST_URI, instead.

This might not be an issue on VIP servers, but should be took into account for local environements. This is filling our local logs as it happens at every frontend request.

Considering that min PHP version is now 7.2, I would suggest something like this:

$uri = $_SERVER['DOCUMENT_URI'] ?? $_SERVER['REQUEST_URI'] ?? null;

// We look to see if a malformed url (represented by 'http:' ) is right after the starting / in DOCUMENT_URI hence position 1
if ( is_404() && ( 1 === strpos( $uri, 'http:' ) || 1 === strpos( $uri, 'https:' ) ) ) {

which is both readable and fix the issue.

An alternative could be using add_query_arg() function so avoid direct access to $_SERVER values at all.

It worth nothing that add_query_arg() uses REQUEST_URI not DOCUMENT_URI.

GaryJones added a commit that referenced this issue Dec 14, 2019
Add support for local development servers that do not have `$_SERVER['DOCUMENT_URI']` defined.

Fixes #976.
GaryJones added a commit that referenced this issue May 30, 2020
Switch `$_SERVER[‘DOCUMENT_URI’]` to `$_SERVER[‘REQUEST_URI’] since that is the more popular key to be supported, particularly in local setups.

Amends logic by searching for `/http:` at position 0 instead of `http:` at position 1 (and likewise for “https:”).

Tidies documentation.

Fixes #976.
brandon-m-skinner pushed a commit that referenced this issue Jun 2, 2020
…local setups (#1381)

* Caching: Improve compatibility with local setups

Switch `$_SERVER[‘DOCUMENT_URI’]` to `$_SERVER[‘REQUEST_URI’] since that is the more popular key to be supported, particularly in local setups.

Amends logic by searching for `/http:` at position 0 instead of `http:` at position 1 (and likewise for “https:”).

Tidies documentation.

Fixes #976.

* Lint fix/style alignment

* Lint fix
@GaryJones
Copy link
Contributor

@gmazzap Thanks for your patience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants