View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0002783 | OXID eShop (all versions) | 4.09. SEO, SEO URL | public | 2011-04-21 17:38 | 2012-12-10 13:34 |
Reporter | tjungcl | Assigned To | |||
Priority | normal | Severity | major | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Product Version | 4.5.0 revision 34568 | ||||
Fixed in Version | 4.5.6 revision 40808 | ||||
Summary | 0002783: SEO decoder decodes history-url into expired-url | ||||
Description | oxseodecoder: - processSeoCall calls _decodeOldUrl - _decodeOldUrl calls _getSeoUrl - _getSeoUrl gets maincat and then gets url from table oxseourl - the select does not check, if oxexpired is 1 => there is a good chance, that a history url is redirected to an expired url. When getting the new seo-url, the sql should get only expired=0 urls. If none found, a new mainlink must be generated and returned. | ||||
Tags | No tags attached. | ||||
Attached Files | _getSeoUrl.txt (1,546 bytes)
// make sure that no expired article-seourl is returned // if no actual seo url is found in oxseo, load article and getMainLink (more expensive, but on the very next call there will be an actual oxseo-entry) protected function _getSeoUrl($sObjectId, $iLang, $iShopId) { $oDb = oxDb::getDb(true); $aInfo = $oDb->getRow( "select oxseourl, oxtype from oxseo where oxobjectid = " . $oDb->quote( $sObjectId ) . " and oxlang = " . $oDb->quote( $iLang ) . " and oxshopid = " . $oDb->quote( $iShopId ) . " order by oxparams limit 1" ); if ('oxarticle' == $aInfo['oxtype']) { $sMainCatId = $oDb->getOne( "select oxcatnid from ".getViewName( "oxobject2category" )." where oxobjectid = " . $oDb->quote( $sObjectId ) . " order by oxtime" ); if ($sMainCatId) { $sUrl = $oDb->getOne( "select oxseourl from oxseo where oxobjectid = " . $oDb->quote( $sObjectId ) . " and oxlang = " . $oDb->quote( $iLang ) . " and oxshopid = " . $oDb->quote( $iShopId ) . " and oxparams = " . $oDb->quote( $sMainCatId ) . " and oxexpired=0" ); //<-- new "and oxexpired=0" if ($sUrl) { return $sUrl; } } // new --> $oArticle = oxNew( "oxarticle" ); $oArticle->setSkipAssign(true); $oArticle->load($sObjectId); $sSeoUrl = $oArticle->getMainLink($iLang); $oStr = getStr(); $sBaseUrl = $this->getConfig()->getShopURL(); if ( $oStr->strpos( $sSeoUrl, $sBaseUrl ) === 0 ) { $sSeoUrl = $oStr->substr( $sSeoUrl, $oStr->strlen( $sBaseUrl ) ); } return $sSeoUrl; // <-- } return $aInfo['oxseourl']; } | ||||
Theme | |||||
Browser | All | ||||
PHP Version | any | ||||
Database Version | any | ||||
related to | 0002827 | resolved | arvydas_vapsva | oxseodecoder->decodeUrl accepts and decodes expired urls without notice |
|
uploaded you my version of _getSeoUrl and marked the new parts (a bit in sql and a bunch of new lines) |
|
@developers: check from source code side if such issue exist and if offered solution can be useful. |
|
your version lacks of object type check.. anyway, please check my comment at https://bugs.oxid-esales.com/view.php?id=2827 |
|
i commented 2827... ... but here i have to say, that its just a bit dirty as it is done now. Decoding an old url into an expired url and then relying on the canonical tag seems just not right. Btw: I dont lack the object type check. It was already in the function: if ('oxarticle' == $aInfo['oxtype']) { ... Also, the loading of the article-object is only done, if there is no actual seourl found. Then (and only then) the url is generated using the articles mainlink-function - once. |
|
Please, at least TRY to get an not expired url $sUrl = $oDb->getOne( "select oxseourl from oxseo where oxobjectid = " . $oDb->quote( $sObjectId ) . " and oxlang = " . $oDb->quote( $iLang ) . " and oxshopid = " . $oDb->quote( $iShopId ) . " and oxparams = " . $oDb->quote( $sMainCatId ) . " order by oxexpired" ); by adding order by oxexpired. If that still returns an expired url, i can do my stuff in an extended decoder myself. |
|
fixed according to last request |