View Issue Details

IDProjectCategoryView StatusLast Update
0003276OXID eShop (all versions)4.07. Source code, Testpublic2012-12-10 13:44
Reportertjungcl Assigned To 
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Product Version4.5.2 revision 38481 
Fixed in Version4.5.7 revision 41909 
Summary0003276: oxaddparams issues
Description- removes deliberaately empty parameters, which makes it impossible to overwrite a parameter with an empty string (which is a valid input value in HTML)
- doesnt check duplicates
- doesnt check multiple &'s

for example:

http://www.test.com/?p1=1&p2=2 plus &p3=3&p1=n1&p2=
Oxaddparams:
=> http://www.test.com/?p1=1&p2=2&&p3=3&p1=n1

Correct:
=> http://www.test.com/?p1=n1&p2=&p3=3
Additional InformationI rewrote the function like this:

function smarty_modifier_oxaddparams( $sUrl, $sDynParams ){


    if ( $sDynParams ) $sUrl .= '?' . $sDynParams; // ?/& doesnt matter. Its exploded with preg anyway

    if (preg_match_all('~(\?|&amp\;|&)([^=|&]+)\=([^&]*)~i', $sUrl, $result)){
        $params = $result[2];
        $values = $result[3];
        $query = array();
        foreach($params as $key => $param) $query[$param] = $values[$key];
        $aUrl = explode('?',$sUrl,2);
        $sUrl = $aUrl[0] . '?' . http_build_query($query,'','&');
    }

    return oxUtilsUrl::getInstance()->processSeoUrl( $sUrl );
}
TagsNo tags attached.
ThemeBoth
BrowserAll
PHP Versionany
Database Versionany

Activities

tjungcl

2011-09-27 13:09

reporter   ~0005271

Last edited: 2011-09-27 13:12

forgot to say: I checked the performance in comparison to the original oxaddparams, and the new function is also a bit faster.

Edit2: The input form replaced a &amp sign in
http_build_query($query,'','& a m p ;');

tjungcl

2011-09-28 17:26

reporter   ~0005272

Last edited: 2011-11-09 10:55

obviously had still some bugs in it (I posted to soon yesterday, wanted to share my findings ;-) )

this seems to work better:

(replace the % in %amp; with &)

function smarty_modifier_oxaddparams( $sUrl, $sDynParams ){

    if ( $sDynParams ) $sUrl .= ( ( strpos( $sUrl, '?' ) !== false ) ? "%amp;":"?" ) . $sDynParams;

    $aUrl = explode('?',$sUrl,2);
    if (count($aUrl) > 1 && preg_match_all('~(^|%amp\;|&)([^=|&]+)\=([^&]*)~i', $aUrl[1], $result)){ //get all param=value pairs from querystring
        $params = $result[2];
        $values = $result[3];
        $aQuery = array();
        $sQuery = "";
        foreach($params as $key => $param) $aQuery[$param] = $values[$key]; //overwrites duplicate params with latest value
        foreach($aQuery as $param => $value) $sQuery .= ($sQuery!=""?"%amp;":"")."$param=$value";
        $sUrl = $aUrl[0] . '?' . $sQuery;
    }

    return oxUtilsUrl::getInstance()->processSeoUrl( $sUrl );
}

Edit: trying to help here. No fun without response...

mindaugas.rimgaila

2011-12-15 12:38

reporter   ~0005497

Implemented URL parameter filtering function, for eliminating duplicate entries by priority.

tjungcl

2011-12-28 10:53

reporter   ~0005531

Thanks, its better, but empty parameters are still deleted, which is wrong behaviour:

http://www.test.com/?p1=1&p2=2 [^] plus &p3=3&p1=n1&p2=

new after bugfix:
=> http://www.test.com/?p1=n1&p2=2&p3=3

Correct would be:
=> http://www.test.com/?p1=n1&p2=&p3=3

tjungcl

2011-12-28 14:11

reporter   ~0005533

Found another problem:

Your new cleanUrlParams function in oxutilsurl replaces brackets in url params, for example:
.../index.php?aproducts[1][am]=5&aproducts[1][aid]=xy

=> .../index.php?aproducts%5B1%5D%5Bam%5D=5&aproducts%5B1%5D%5Baid%5D=xy

Thats why I didnt use http_build_query but built the string myself (as you can see, I changed that from my first to the second suggestion).