Unexpected permanent redirects when used in conjunction with Redirect module

Created on 13 April 2023, over 1 year ago

Problem/Motivation

The beta3 release of the Cloudflare module changed the priority at which the request event is modified. In 1.0.0-beta2, the ClientIpRestore event subscriber had a priority of 20, which fires after the subscriber in the Redirect module. In 1.0.0-beta3, the CloudFlareMiddleware has a priority of 300, so it fires before the redirect module's event subscriber (as it should, right?).

I think that the problem lies in that the Cloudflare's call to Request::overrideGlobals modifies $_SERVER['QUERY_STRING'] such that the query string no longer matches what the user sent over, and introduces a mismatch between Request::server->get('QUERY_STRING') versus Request::getRequesturi. ?query-var !== ?query-var=.

Later on in the processing (priority=30), the Redirect module accesses $request->server->get('QUERY_STRING') to get the query string, which yields ?query-var=, but also calls $request->getRequestUri() which yields ?query-var.

This results in a mismatch that causes the redirect module to issue a 301 from https://example.com?query-var to https://example.com?query-var=.

on its own, it might be innocuous enough, but...

This change is proving to be somewhat troublesome when combined with the page_cache_query_ignore module.

  1. The Cloudflare module hijacks / overrides the superglobal $_SERVER via a call to Request::overrideGlobals on line 175 of the middleware (this is what transforms the current query parameter from some-query-param to some-query-param=
  2. The Redirect module picks up a route like https://example.com?some-query-param and redirects it to https://example.com?some-query-param=
  3. The Page Cache module (which has been decorated by Page Cache Query Ignore) doesn't vary between some-query-param and some-query-param=, and therefore responds with a HTTP/301 from https://example.com?some-query-param= to https://example.com?some-query-param=, thus forming an infinite loop.

Proposed resolution

Prevent the Cloudflare module from altering $_SERVER['QUERY_STRING'].

Remaining tasks

Needs manual testing.

  1. Install Redirect + Cloudflare
  2. Visit https://example.com?query-parameter, observe that the user is 301 redirected to https://example.com?query-parameter=
  3. Apply patch
  4. Visit https://example.com?query-parameter, observe that the user is not 301 redirected

User interface changes

None

API changes

None

Data model changes

None

πŸ› Bug report
Status

Needs review

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States luke.leber Pennsylvania

Live updates comments and jobs are added and updated live.
  • Needs manual testing

    The change/bugfix cannot be fully demonstrated by automated testing, and thus requires manual testing in a variety of environments.

Sign in to follow issues

Comments & Activities

Production build 0.71.5 2024