Ultimate SEO v3 (and v2.200) [Support Thread]

John Large:

Wondering if anyone can help.

I've been using this add-onn for as long as I can remember & never had an issue.

Last year after a long migration process we upgraded from a fairly old version of ZC to the then current 1.5.6c. Everything went fine with the upgrade and the site has been working well. This last month Google has started highlighting redirect loop errors for deleted products. After having a dig through the settings, it would appear that if I enable automatic rewrites in utlimate URLS & then try to visit the url for a now deleted product, it will fire a redirect loop in firefox. This confirms the behaviour I'm seeing in search console.

if I disable automatic rewrites, the "sorry, product not found" page will show and after a 302 redirect will show a 404 header.

I've used the automatic redirects in ultimate urls for years & never noticed a problem, but for some reason it is now not playing ball. I don't know where to start. My .htaccess is the same as when it did work on the older version of Zen Cart. I've compared the two and all ultimate URLS rewrites are present & correct. For now I've disabled automatic rewrites & will let google uses the canonical/nofollows generated by zen cart. The changes include the newer version of ZC & now running on php 7.3.27

Can anyone point me in the right direction? I tried uninstaling ultimate URLS completely & have freshly installed version 3.0.8 from numinix website. Still can't get it to work as expected. Any help would be greatly appreciated.

Thanks, John.

Didn't mean numinix website, that was for easy google anlytics plugin I also upgraded. Apologies, 3.0.8 of this plugin was installed directly from the ZC plugins page.

Confirmed bug :oops:. I've opened the following GitHub issue to track the correction: https://github.com/lat9/usu/issues/42

lat9:

Confirmed bug :oops:. I've opened the following GitHub issue to track the correction: https://github.com/lat9/usu/issues/42

Well that makes me feel a lot better after going around in circles trying to figure out what I'd done wrong. Glad I posted. Hope it's not too dificult a fix. :smile:

John Large:

Well that makes me feel a lot better after going around in circles trying to figure out what I'd done wrong. Glad I posted. Hope it's not too dificult a fix. :smile:
Well, it only took me 3 hours to figure out what the heck is going on! I'll call that difficult.:blink:

The underlying issue is the (unwanted) help provided by /includes/init_includes/init_sanitize.php when an invalid products_id variable is included in the URL. That module, for the current v3.0.8 and earlier versions of USU, is loaded prior to the USU intercept of calls to the zen_href_link function. When that module finds an invalid products_id, it performs a redirect to main_page=product_info&products_id=xxx ... which doesn't match the USU-generated link for that invalid product.

That mismatch, once USU gets activated, causes USU to issue a redirect to its 'proper' URL form which results in init_sanitize (on the redirect) to redirect to the base form ... you get the drift.

The update's going to be a tad complicated due to prerequisites, since the base language session variables aren't (by default) loaded until later in the start-up processing. Essentially, the USU start-up will now load itself prior to that init_sanitize module's load and will also register those base language variables (via init_languages.php) as part of its start-up.

I should have a beta version of USU v3.0.9 available shortly for 3rd party testing.

v3.0.9-beta1 now available for download: https://github.com/lat9/usu/releases/tag/v3.0.9-beta1

I'll plan to make this a formal v3.0.9 release early next week unless I receive feedback that the correction doesn't work.

lat9:

v3.0.9-beta1 now available for download: https://github.com/lat9/usu/releases/tag/v3.0.9-beta1

I'll plan to make this a formal v3.0.9 release early next week unless I receive feedback that the correction doesn't work.

Thanks lat9.

Invalid or deleted product ids are now redirecting to the product not found page & sending a 404.

One problem is, old urls arent rewriting to new ones. For example if originally my product was named and available at https://www.koolbadges.co.uk/japanese-doll-badges-p-3121.html but after being renamed is now at https://www.koolbadges.co.uk/cute-blue-japanese-doll-badges-p-3121.html with the rewrite I would expect the old url to redirect to the new to reflect the name change, however both urls are returning a 200 response and are valid.

John Large:

Thanks lat9.

Invalid or deleted product ids are now redirecting to the product not found page & sending a 404.

One problem is, old urls arent rewriting to new ones. For example if originally my product was named and available at https://www.koolbadges.co.uk/japanese-doll-badges-p-3121.html but after being renamed is now at https://www.koolbadges.co.uk/cute-blue-japanese-doll-badges-p-3121.html with the rewrite I would expect the old url to redirect to the new to reflect the name change, however both urls are returning a 200 response and are valid.
@John Large, I'm guessing that the 'failure to redirect' is after applying the changes posted in GitHub? Please advise.

Yes.

The failure to redirect is after installing the beta. You can potentially write any url string to create links as long as the product id is val, and it will return a 200 response. I'm not sure if this has always been the case with the plugin, it's not something I've tested before, but since google started complaing about our website I've been digging into it. Possibly better to 301 any requests where the url string doesn’t match the canonical to the canonical address for duplicate content/seo benefits?

John Large:

Yes.

The failure to redirect is after installing the beta. You can potentially write any url string to create links as long as the product id is val, and it will return a 200 response. I'm not sure if this has always been the case with the plugin, it's not something I've tested before, but since google started complaing about our website I've been digging into it. Possibly better to 301 any requests where the url string doesn’t match the canonical to the canonical address for duplicate content/seo benefits?

Just updated on github, but will put it here for anyone else following this thread.

Just tested further, if I manually enter a url in the old format such as

https://www.koolbadges.co.uk/index.php?main_page=product_info&products_id=904

It now doesn't redirect with the beta to the SEO url. That will return a 200 response. The rewritten url is https://www.koolbadges.co.uk/camo-pink-badges-p-904.html?cPath=9 so the 301 automatic rewrites now don't seem to be working.

Reverted to 3.0.8 and the redirects work as I mentioned, redirecting any changes in the url to the canonical, so the automatic rewrites appear to have been broken by the changes in the beta. It would seem that by fixing the redirect loop on deleted products it has broken the automatic rewrites on valid products

@lat9

in commit #42, in the usu class, it seems you have deleted line 100 which had the following code:

// Determine canonical (if needed)
        $this->canonical();

was this on purpose?

in my testing, this changes causes the automatic redirects to not happen.

now, whether that line of code is pertinent to the original redirect problem, i can not say, as i did not reproduce the original error.

john,
what happens if you add that line back?

starting at line 94 of includes/classes/usu.php, make it look like so:

            } elseif (IS_ADMIN_FLAG) { is was is causing the
                // Message Stack will be available when loaded from the admin
                $GLOBALS['messageStack']->add(sprintf(USU_PLUGIN_WARNING_TABLE, TABLE_USU_CACHE), 'warning');
            }
        }

        // Determine canonical (if needed)
        $this->canonical();

        // Start logging
        $this->debug = false;
        if (defined('USU_DEBUG') && USU_DEBUG == 'true') {

does this fix your redirect as well as preserve the fix to your original problem?

carlwhat:

@lat9

in commit #42, in the usu class, it seems you have deleted line 100 which had the following code:

// Determine canonical (if needed)
$this->canonical();

> 
> was this on purpose?  
> 
> in my testing, this changes causes the automatic redirects to not happen.
> 
> now, whether that line of code is pertinent to the original redirect problem, i can not say, as i did not reproduce the original error.
> 
> john,
> what happens if you add that line back?
> 
> starting at line 94 of includes/classes/usu.php, make it look like so:
> 
> ```php
            } elseif (IS_ADMIN_FLAG) { is was is causing the
                // Message Stack will be available when loaded from the admin
                $GLOBALS['messageStack']->add(sprintf(USU_PLUGIN_WARNING_TABLE, TABLE_USU_CACHE), 'warning');
            }
        }

        // Determine canonical (if needed)
        $this->canonical();

        // Start logging
        $this->debug = false;
        if (defined('USU_DEBUG') && USU_DEBUG == 'true') {

does this fix your redirect as well as preserve the fix to your original problem?

Can confirm that if this line is added back in, it fixes the 301 redirects for products that exist but introduces back the redirection loop for deleted products. You can test it yourself, visit a product on your website & add a few numbers to the product id to create a product that doesn't exist. With that line added back, you will/should get the redirection loop. Without that line the loop is fixed, but the 301's are broken for canonicals.

John Large:

Can confirm that if this line is added back in, it fixes the 301 redirects for products that exist but introduces back the redirection loop for deleted products.
:thumbsup:
John Large:

You can test it yourself, visit a product on your website & add a few numbers to the product id to create a product that doesn't exist. With that line added back, you will/should get the redirection loop. Without that line the loop is fixed, but the 301's are broken for canonicals.
:rotflmao:
who knew?

lat9:

Well, it only took me 3 hours to figure out what the heck is going on! I'll call that difficult.:blink:

The underlying issue is the (unwanted) help provided by /includes/init_includes/init_sanitize.php when an invalid products_id variable is included in the URL. That module, for the current v3.0.8 and earlier versions of USU, is loaded prior to the USU intercept of calls to the zen_href_link function. When that module finds an invalid products_id, it performs a redirect to main_page=product_info&products_id=xxx ... which doesn't match the USU-generated link for that invalid product.

That mismatch, once USU gets activated, causes USU to issue a redirect to its 'proper' URL form which results in init_sanitize (on the redirect) to redirect to the base form ... you get the drift.

The update's going to be a tad complicated due to prerequisites, since the base language session variables aren't (by default) loaded until later in the start-up processing. Essentially, the USU start-up will now load itself prior to that init_sanitize module's load and will also register those base language variables (via init_languages.php) as part of its start-up.

I should have a beta version of USU v3.0.9 available shortly for 3rd party testing.

yeah, this problem has been around for a while. on my clients sites, i just comment out line 105 of the includes/init_includes/init_sanitize.php, ie:

      /**
       * do not recheck redirect
       */
      $_SESSION['check_valid_prod'] = false;
      //zen_redirect(zen_href_link($_GET['main_page'], 'products_id=' . $_GET['products_id']));
    }

not ideal, but gets the job done.

best.

carlwhat:

:thumbsup:

:rotflmao:
who knew?

yeah, this problem has been around for a while. on my clients sites, i just comment out line 105 of the includes/init_includes/init_sanitize.php, ie:

  /**
   * do not recheck redirect
   */
  $_SESSION['check_valid_prod'] = false;
  //zen_redirect(zen_href_link($_GET['main_page'], 'products_id=' . $_GET['products_id']));
}

> 
> not ideal, but gets the job done.
> 
> best.

Is that safe to do? I'm assuming the sanitzie functions are there to stop scripting and other such things in the url (could be wrong, not soemthign I've ever dug into). Also, does it break anything else that you have noticed?

I've just checked my own init_sanitize.php and dont have that line of code in mine.

John Large:

Is that safe to do? I'm assuming the sanitize functions are there to stop scripting and other such things in the url (could be wrong, not soemthign I've ever dug into). Also, does it break anything else that you have noticed?

I've just checked my own init_sanitize.php and dont have that line of code in mine.

Well on my version of 1.5.6c (which seems to use a different version of init_sanitize.php) I've just edited out around line 140 and reverted USU to 3.0.8 and now both the 404's and the 301 redirects work.

 * do not recheck redirect
       */
      $_SESSION['check_valid'] = 'false';
      //commented out by john
      //zen_redirect(zen_href_link($_GET['main_page'], 'products_id=' . $_GET['products_id']));
    }

My only question, is this a safe way to acheive this? Will it stop urls being sanitized and leave us more vulnerable?

John Large:

Is that safe to do? I'm assuming the sanitzie functions are there to stop scripting and other such things in the url (could be wrong, not soemthign I've ever dug into). Also, does it break anything else that you have noticed?

I've just checked my own init_sanitize.php and dont have that line of code in mine.
I wouldn't suggest that change, since I'm *assuming *that it's there for a good reason. Let me rework the redirect-loop changes to get the USU/Zen Cart integration working properly.

John Large:

Is that safe to do? I'm assuming the sanitzie functions are there to stop scripting and other such things in the url (could be wrong, not soemthign I've ever dug into). Also, does it break anything else that you have noticed?

I've just checked my own init_sanitize.php and dont have that line of code in mine.

john,
sorry i did not notice you are on 156c. that file should look like this one:

https://github.com/zencart/zencart/blob/v156/includes/init_includes/init_sanitize.php

the line you need to comment out is line 140.

as far as safe to do? i think so. it makes it problematic when you upgrade as you need to remember what you did; but i have that running on clients sites for quite some time with no ill effect.

if @lat9 comes up with a more elegant solution that does not involve changing ZC core code, i am all for it. i know i spent many hours myself looking at it (at one time), and this just seemed to be the easiest solution.

as far as @lat9s comment, total respect, but that code has been there for 9 years. i see no good reason for it, and again, empirically, i see no ill effect.

best.

lat9:

I wouldn't suggest that change, since I'm *assuming *that it's there for a good reason. Let me rework the redirect-loop changes to get the USU/Zen Cart integration working properly.

lets look at the code for a quick second, and specifically this var $_SESSION['check_valid_prod']:

/**
 * validate products_id for search engines and bookmarks, etc.
 */
  if (isset($_GET['products_id']) && (!isset($_SESSION['check_valid_prod']) || $_SESSION['check_valid_prod'] != false)) {
    $check_valid = zen_products_id_valid($_GET['products_id']) && !empty($_GET['main_page']);
    if (!$check_valid) {
      $_GET['main_page'] = zen_get_info_page($_GET['products_id']);
      /**
       * do not recheck redirect
       */
      $_SESSION['check_valid_prod'] = false;
      zen_redirect(zen_href_link($_GET['main_page'], 'products_id=' . $_GET['products_id']));
    }
  }

the redirect is only suppose to happen once! we are first checking if that session var is set and not false. if so, lets set it to false, and then redirect. now the next time here, we should NOT hit the redirect as the session var is now false. but empirically we know that is happening.

while done with good intentions, this bit of code seems adrift to me. and has been for years.

but i am completely open to another opinion on it.

carlwhat:

@lat9

in commit #42, in the usu class, it seems you have deleted line 100 which had the following code:

// Determine canonical (if needed)
$this->canonical();

> 
> was this on purpose?  
> 
> in my testing, this changes causes the automatic redirects to not happen.
> 
> now, whether that line of code is pertinent to the original redirect problem, i can not say, as i did not reproduce the original error.
> 
> john,
> what happens if you add that line back?
> 
> starting at line 94 of includes/classes/usu.php, make it look like so:
> 
> ```php
            } elseif (IS_ADMIN_FLAG) { is was is causing the
                // Message Stack will be available when loaded from the admin
                $GLOBALS['messageStack']->add(sprintf(USU_PLUGIN_WARNING_TABLE, TABLE_USU_CACHE), 'warning');
            }
        }

        // Determine canonical (if needed)
        $this->canonical();

        // Start logging
        $this->debug = false;
        if (defined('USU_DEBUG') && USU_DEBUG == 'true') {

does this fix your redirect as well as preserve the fix to your original problem?
Noting that the call to $this->canonical in the usu class' constructor was moved (as part of the redirect-loop correction) to the USU's observer class upon receipt of a notification issued at the beginning of the init_canonical.php's processing.

The subsequent issue caused by that move is that the redirect handling for 'renamed' products/categories needs to move also to the canonical-link generation phase of a page's rendering.

carlwhat:

lets look at the code for a quick second, and specifically this var $_SESSION['check_valid_prod']:

/**

• validate products_id for search engines and bookmarks, etc.
  /
  if (isset($_GET['products_id']) && (!isset($_SESSION['check_valid_prod']) || $_SESSION['check_valid_prod'] != false)) {
  $check_valid = zen_products_id_valid($_GET['products_id']) && !empty($_GET['main_page']);
  if (!$check_valid) {
  $_GET['main_page'] = zen_get_info_page($_GET['products_id']);
  /*
  * do not recheck redirect
  */
  $_SESSION['check_valid_prod'] = false;
  zen_redirect(zen_href_link($_GET['main_page'], 'products_id=' . $_GET['products_id']));
  }
  }

> 
> the redirect is only *suppose to happen once!*   we are first checking if that session var is set and not false.  if so, lets set it to false, and then redirect.  now the next time here, we should **NOT** hit the redirect as the session var is now false.  but empirically we know that is happening.
> 
> while done with good intentions, this bit of code seems adrift to me.  and has been for years.  
> 
> but i am completely open to another opinion on it.
@carlwhat, the more I look at that section of code, the more I wonder about it.

1. We're looking for a link with a products_id variable (once per session), so if someone's pounding on a site with multiple links all with invalid products_id values, we're only going to see the first one?

2. We've found a products_id that isn't valid ... so we're going to use that invalid value to look up its non-existent _info page and use that known invalid products_id as part of the redirection link?

I've got a headache ...

lat9:

@carlwhat, the more I look at that section of code, the more I wonder about it.

1. We're looking for a link with a products_id variable (once per session), so if someone's pounding on a site with multiple links all with invalid products_id values, we're only going to see the first one?

2. We've found a products_id that isn't valid ... so we're going to use that invalid value to look up its non-existent _info page and use that known invalid products_id as part of the redirection link?

I've got a headache ...

yaa, i feel your pain. as i looked at this closer, i remember burning many hours on it many moons ago. pretty clear it does not work (or i could not find a solution) to me.

i've been running with that line commented out for a minimum of 6 years. that redirect code was in there starting 9 years ago (at least as far as i can tell from github).

perhaps off topic, but what does this have to do with sanitizing vars? is that not the script we are in?

:huh: