PagePathHistory (Core) loses QueryString when redirecting

[flipzoom]

Description

The PagePath History module does not check if a QueryString parameter exists and redirects it without it. Example:

Old PagePath: /dhdl/?filter=1&filter=2
New PagePath: /products/

It is redirected to /products/, but without QueryString parameters.

Suggestion for a possible fix in PagePathHistory.module

Add the following in line 289

// Checks if QueryString exists
$queryString = ($this->getQueryString()) ?: "";

// Redirect
$this->session->redirect($page->url.$queryString);

And add the following new method in line 297

/**
  * Checks if QueryString exists. If so, these are added to the current redirect.
  * 
  * @return String or Boolean
  */
public function getQueryString() {

	// Init QueryString array
	$queryParameter	= array();

	// Parse current QueryString
	parse_str($_SERVER['QUERY_STRING'], $queryParameter);

	// Remove PW system QueryParameter
	unset($queryParameter['it']);

	// Return QueryString or false
	return (count($queryParameter) >= 1) ? "?".http_build_query($queryParameter) : false;
}

Setup/Environment

• ProcessWire version: 3.0.37
• PHP version: 7.0.12
• MySQL version: 5.5.53-MariaDB

[ryancramerdesign]

Thanks for the report. I understand the inconvenience for this particular case, but as a matter of safety, I specifically avoid passing through unknown user input variables in http headers (like redirects). I would suggest using an Apache redirect for this purpose if possible.

[flipzoom]

Okay, I understand the problem. How about an option in the PagePathHistory module? Allow or disable QueryString redirection and perhaps define the allowed parameters in a list?

[isellsoap]

@FlipZoomMedia Your suggestion seems to be a perfect fit for the requests repository. Fell free to open an issue there. Thank you.