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 |