Skip to content

Commit

Permalink
#42: Correcting initial redirect, don't create alternate URLs if para…
Browse files Browse the repository at this point in the history 
…meters have 'issues'.
  • Loading branch information
@lat9
lat9 committed Apr 20, 2021
1 parent c96c27f commit 2adeec5
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 23 deletions.
89 changes: 67 additions & 22 deletions includes/classes/usu.php
View file
Expand Up @@ -24,6 +24,7 @@ class usu

protected $cache,
$languages_id,
$parameters_valid,
$reg_anchors,
$cache_file,
$uri,
Expand Down Expand Up @@ -108,13 +109,6 @@ function __construct($languages_id = '')
$this->logpage = (isset($_GET['main_page'])) ? $_GET['main_page'] : 'index';
}
}

// Redirect if enabled and neccessary
if (defined('USU_REDIRECT') && USU_REDIRECT == 'true') {
if ($this->needs_redirect()) {
$this->redirect();
}
}
}

/**
Expand Down Expand Up @@ -166,7 +160,6 @@ public function href_link($page = '', $parameters = '', $connection = 'NONSSL',
// of claiming a link is "static" when it is not. So we ignore the value
// of "static" if the page starts with "index.php?"
if (strstr($page, 'index.php?') !== false) {

// If we find the main_page parse the URL
$result = array();
if (preg_match('/[?&]main_page=([^&]*)/', $page, $result) === 1) {
Expand Down Expand Up @@ -219,6 +212,14 @@ public function href_link($page = '', $parameters = '', $connection = 'NONSSL',
}
}

// -----
// Indicate that, so far, no issues have been found with the link's
// parameters. That might be changed during the parse_parameters method's
// processing, if an invalid products_id, cPath or EZ-Pages' id parameter is
// found.
//
$this->parameters_valid = true;

// We start with no separator, so define one.
$separator = '?';
if (zen_not_null($parameters)) {
Expand All @@ -227,6 +228,14 @@ public function href_link($page = '', $parameters = '', $connection = 'NONSSL',
$link .= (($page != FILENAME_DEFAULT && $page != '') ? $page . USU_END : '');
}

// -----
// If the parameters supplied for the link aren't valid, don't regenerate a USU-formatted
// link.
//
if (!$this->parameters_valid) {
return null;
}

$link = $this->add_sid($link, $add_session_id, $connection, $separator);

// -----
Expand Down Expand Up @@ -315,8 +324,8 @@ protected function parse_parameters($page, $params, &$separator)
// Cleanup parameters and convert to initial array
$params = trim($params, "?& \t\n\r\0\x0B");
$p = array();
if (!empty($params)) {
$p = @explode('&', $params);
if (!empty($params) && is_string($params)) {
$p = explode('&', $params);
}

// Add the cPath to the start of the parameters if present
Expand Down Expand Up @@ -601,14 +610,22 @@ protected function get_product_name($pID, $cID = null)
default:
$pName = $this->filter(zen_get_products_name($pID));

if (USU_FORMAT == 'parent' && USU_CATEGORY_DIR == 'off') {
$masterCatID = (int)zen_get_products_category_id($pID);
$category_id = ($cID !== null ? $cID : $masterCatID);
$pName = $this->get_category_name($category_id, 'original') . '-' . $pName;
}
// -----
// If the products's name wasn't found, indicate that there's a link
// parameter issue so that the requested URL won't be generated.
//
if ($pName === '') {
$this->parameters_valid = false;
} else {
if (USU_FORMAT == 'parent' && USU_CATEGORY_DIR == 'off') {
$masterCatID = (int)zen_get_products_category_id($pID);
$category_id = ($cID !== null ? $cID : $masterCatID);
$pName = $this->get_category_name($category_id, 'original') . '-' . $pName;
}

if (is_array($this->cache)) {
$this->cache['PRODUCTS'][$pID] = $pName;
if (is_array($this->cache)) {
$this->cache['PRODUCTS'][$pID] = $pName;
}
}
$return = $pName;
break;
Expand Down Expand Up @@ -657,7 +674,6 @@ protected function get_product_canonical($pID)
//
$pName = zen_get_products_name($pID);
if (!empty($pName)) {

$masterID = zen_get_products_category_id($pID);
$retval = $this->get_category_name($masterID) . $this->reg_anchors['cPath'] . $masterID . '/';

Expand Down Expand Up @@ -748,7 +764,13 @@ protected function get_category_name(&$cID, $format = USU_FORMAT)
}
}

if ($cName !== '' && is_array($this->cache)) {
// -----
// If the category's name wasn't found, indicate that there's a link
// parameter issue so that the requested URL won't be generated.
//
if ($cName === '') {
$this->parameters_valid = false;
} elseif (is_array($this->cache)) {
$this->cache['CATEGORIES'][$full_cPath] = $cName;
}
$return = $cName;
Expand Down Expand Up @@ -786,7 +808,13 @@ protected function get_manufacturer_name($mID)
$result = $db->Execute($sql, false, true, 43200);
$mName = ($result->EOF) ? '' : $this->filter($result->fields['mName']);

if ($mName !== '' && is_array($this->cache)) {
// -----
// If the manufacturer's name wasn't found, indicate that there's a link
// parameter issue so that the requested URL won't be generated.
//
if ($mName === '') {
$this->parameters_valid = false;
} elseif (is_array($this->cache)) {
$this->cache['MANUFACTURERS'][$mID] = $mName;
}
$return = $mName;
Expand Down Expand Up @@ -845,7 +873,13 @@ protected function get_ezpages_name($ezpID)
$result = $db->Execute($sql, false, true, 43200);
$ezpName = ($result->EOF) ? '' : $this->filter($result->fields['ezpName']);

if ($ezpName !== '' && is_array($this->cache)) {
// -----
// If the EZ-Page's name wasn't found, indicate that there's a link
// parameter issue so that the requested URL won't be generated.
//
if ($ezpName === '') {
$this->parameters_valid = false;
} elseif (is_array($this->cache)) {
$this->cache['EZPAGES'][$ezpID] = $ezpName;
}
$return = $ezpName;
Expand Down Expand Up @@ -1432,6 +1466,13 @@ public function canonical()
}
unset($product_page);
}

// Redirect if enabled and necessary
if (defined('USU_REDIRECT') && USU_REDIRECT == 'true') {
if ($this->needs_redirect()) {
$this->redirect();
}
}
}

/**
Expand Down Expand Up @@ -1503,8 +1544,12 @@ protected function needs_redirect()
return true;
}
} else {
// -----
// If the requested URI contained invalid per-page parameters, e.g. an invalid 'id'
// for an EZ-page request, the request is not redirected.
//
$this->log('Generated path did not match the requested URI.' . PHP_EOL . json_encode($parsed_uri) . PHP_EOL . json_encode($this->redirect_uri));
return true;
return $this->parameters_valid;
}
} else {
// See if the parameters match. We do not care about order.
Expand Down
4 changes: 3 additions & 1 deletion readme/index.html
View file
Expand Up @@ -254,12 +254,14 @@ <h3 class="list-header">Uninstalling / Removal</h3>
<div id="changelog">
<h2>Change History</h2>
<ul>
<li>v3.0.9, 2021-04-18 (lat9):<ul>
<li>v3.0.9, 2021-04-20 (lat9):<ul>
<li>BUGFIX: Correct redirect-loop detected by browsers when an invalid 'products_id' is present in the URL.</li>
<li>BUGFIX: Changes to products, categories, manufacturers and EZ-pages not properly cached during admin processing.</li>
<li>The following files were changed:<ol>
<li>/includes/auto_loaders/config.usu.php</li>
<li>/includes/classes/usu.php</li>
<li>/includes/classes/observers/UsuObserver.php</li>
<li>/YOUR_ADMIN/includes/classes/observers/UsuAdminObserver.php</li>
<li>/YOUR_ADMIN/includes/init_includes/init_usu_admin.php</li>
</ol></li>
</ul></li>
Expand Down

2 comments on commit 2adeec5

@proseLA
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the redirect was getting done prior to some sort of validation? and moving the redirect to after the validation into the canonical function was the answer?

i will try and test it later, but bravo! well done!

👍 👍

@lat9
Copy link
Owner Author

@lat9 lat9 commented on 2adeec5 Apr 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That redirect was (in prior versions) being done in the usu class-constructor, after its call to generate the canonical link.

The canonical generation is now 'called for' within the storefront's USU observer, upon receiving notification from the init_canonical.php module's processing. Moving the call to determine whether/not the redirect is needed into the canonical method provides the same sequencing now, just in a different spot.

Please sign in to comment.