Skip to content

Instantly share code, notes, and snippets.

Last active August 11, 2022 17:27
Show Gist options
  • Save edannenberg/5310008 to your computer and use it in GitHub Desktop.
Save edannenberg/5310008 to your computer and use it in GitHub Desktop.
Fixes the catalog url rewrite indexer in Magento 1.7.x-1.9.x See for details.Update: DexterDee in the ticket above noted that the previous patch had some side effects. This updated patch still feels like duct tape but at least it seems to be free of the mentioned side effects. It also fix…
diff -rupN mage_org/app/code/core/Mage/Catalog/Model/Url.php src_shop/app/code/core/Mage/Catalog/Model/Url.php
--- mage_org/app/code/core/Mage/Catalog/Model/Url.php 2013-11-19 00:48:25.679009391 +0100
+++ src_shop/app/code/core/Mage/Catalog/Model/Url.php 2013-11-19 00:49:24.188005601 +0100
@@ -643,13 +643,24 @@ class Mage_Catalog_Model_Url
$this->_rewrite = $rewrite;
return $requestPath;
+ // avoid unnecessary creation of new url_keys for duplicate url keys
+ $noSuffixPath = substr($requestPath, 0, -(strlen($suffix)));
+ $regEx = '#^('.preg_quote($noSuffixPath).')(-([0-9]+))?('.preg_quote($suffix).')#i';
+ $currentRewrite = $this->getResource()->getRewriteByIdPath($idPath, $storeId);
+ if ($currentRewrite && preg_match($regEx, $currentRewrite->getRequestPath(), $match)) {
+ $this->_rewrite = $currentRewrite;
+ return $currentRewrite->getRequestPath();
+ }
// match request_url abcdef1234(-12)(.html) pattern
$match = array();
$regularExpression = '#^([0-9a-z/-]+?)(-([0-9]+))?('.preg_quote($suffix).')?$#i';
if (!preg_match($regularExpression, $requestPath, $match)) {
return $this->getUnusedPath($storeId, '-', $idPath);
- $match[1] = $match[1] . '-';
+ $match[1] = $noSuffixPath . '-'; // always use full prefix of url_key
+ unset($match[3]); // don't start counting with a possible number in the url_key
$match[4] = isset($match[4]) ? $match[4] : '';
$lastRequestPath = $this->getResource()
Copy link

nkajic commented Jan 17, 2017

Hi, just to confirm this is still a valid fix for Magento ver. Can't belive this is still a issue in CE Thx!

Copy link

tpaaro commented May 8, 2018

This patch has a problem when the variable $noSuffixPath is empty. Magento will create faulty URL rewrites cause of that. Those faulty rewrites will consist of two elements: a dash (-) and a number. E.g. -12345.

I recommend to modify line 25:
$match[1] = $noSuffixPath . '-'; => $match[1] = ($noSuffixPath ?: $match[1]) . '-';

So incase if the $noSuffixPath variable is empty, it would use default value set in $match[1].

Otherwise this patch works and avoids creating duplicate URL rewrites (CE

Copy link

It's worth noting that Magento CE makes some major changes around this code. I'm evaluating if it fixes the underlying issue so that this workaround isn't necessary anymore.

Copy link

@ajbonner, Thanks for the heads up, any update on your findings?

Copy link

It seems it was fixed on

Copy link

diamondavocado commented Feb 27, 2020

It took them over five years to fix this (serious) issue. If anyone wants an example of why you shouldn't use Magento, here's one.

Edit: I see they also deleted the bug report and their forums, as an extra "fuck you".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment