SquirrelMail

This bug has been resolved.

Please pull the latest version from the appropriate development
tree CVS to fix your bug.

Thank you for your help in resolving this issue.

Second version passed all tests. But my tests contain only correctly
formated urls and two calls that break things. Currently I am not testing
for any broken urls.

#1 and #13 are good catches :)
Here we are, version 2 :

-------8<----------8<---------[ functions/html.php ]---8<--------
function set_url_var($url, $var, $val=0, $link=true) {
$url = str_replace('&amp;','&',$url);

if (strpos($url, '?') === false) {
$url .= '?';
}

list($uri, $params) = explode('?', $url, 2);

$newpar = array();
$params = explode('&', $params);

foreach ($params as $p) {
if (trim($p)) {
$p = explode('=', $p);
$newpar[$p[0]] = (isset($p[1]) ? $p[1] : '');
}
}

if ($val === 0) {
unset($newpar[$var]);
} else {
$newpar[$var] = $val;
}

if (!count($newpar)) {
return $uri;
}

$url = $uri . '?';
foreach ($newpar as $name => $value) {
$url .= "$name=$value&";
}

$url = substr($url, 0, -1);
if ($link) {
$url = str_replace('&','&amp;',$url);
}

return $url;
}
-------8<----------8<----------8<----------8<----------8<--------

Let's test it thoroughly, since there are obvious pitfalls in this
kind of functions. A second test suite should include incorrect and
misformatted URL as well (eg &novalue&question=problem?&&fail= )

I agree the behavious with zero is confusing, but I'm afraid changing
it might break things. Using false or null is more suited to a strong
type checking.

* add variable to url without parameters - Failed
Expected: test.php?test=value
Result: test.php?=&test=value

* add variable to url without parameters (link=true) - Failed
Expected: test.php?test=value
Result: test.php?=&amp;test=value

* add variable to url with parameters - OK
* add variable to url with parameters (link=true) - OK

* replace first and only parameter - OK
* replace first and only parameter (link=true) - OK

* replace first parameter on url with more than one parameter - OK
* replace first parameter on url with more than one parameter (link=true)
- OK

* replace second parameter on url with more than one parameter - OK
* replace second parameter on url with more than one parameter (link=true)
- OK

* replace last parameter on url with more than one parameter - OK
* replace last parameter on url with more than one parameter (link=true) -
OK

* delete the only parameter - Failed
Expected: test.php
Result: test.ph

* delete the only parameter (link=true) - Failed
Expected: test.php
Result: test.ph

* delete first parameter - OK
* delete first parameter (link=true) - OK

* delete second parameter - OK
* delete second parameter (link=true) - OK

* delete last parameter - OK
* delete last parameter (link=true) - OK

* set value to 0 - OK
* replace first parameter with partial name match in other params - OK

> "remember that 0 will delete a variable, '0' will set it to 0"

PHP developers are not used to strict variable types. They will have to
learn it only in some PHP6 setups. Zero string and zero integer are usually
the same thing. http://www.php.net/language.types.null

The set_url_var needs a complete rewrite to be more robust and failsafe.
Try with the following version :

-------8<----------8<---------[ functions/html.php ]---8<--------
function set_url_var($url, $var, $val=0, $link=true) {
$url = str_replace('&amp;','&',$url);

if (strpos($url, '?') === false) {
$url .= '?';
}

list($uri, $params) = explode('?', $url, 2);

$newpar = array();
$params = explode('&', $params);

foreach ($params as $p) {
$p = explode('=', $p);
$newpar[$p[0]] = (isset($p[1]) ? $p[1] : '');
}

if ($val === 0) {
unset($newpar[$var]);
} else {
$newpar[$var] = $val;
}

$url = $uri . (count($newpar) ? '?' : '');

foreach ($newpar as $name => $value) {
$url .= "$name=$value&";
}

$url = substr($url, 0, -1);
if ($link) {
$url = str_replace('&','&amp;',$url);
}

return $url;
}
-------8<----------8<----------8<----------8<----------8<--------

set_url_var('test.php?test=value','test','value2'); .... OK
set_url_var('test.php?test=value&test2=value2','test',0,false); ... OK
(test is deleted)
set_url_var('test.php?test=value&test2=value2','test','0',false); ... OK
(test is set to 0)
set_url_var('test.php?test=value1&second_test=value1&third_test=value1','test','replaced1',false);
... OK

Try to see if everything works good (remember that 0 will delete a
variable, '0' will set it to 0)
If no problem arise, the fix will be commited to the SVN.

The issue is confirmed and attended to right now.
Thank you for your bug report. A fix should be available soon.

4.
set_url_var('test.php?test=value1&second_test=value1&third_test=value1','test','replaced1',false);

Expected: test.php?test=replaced1&second_test=value1&third_test=value1

Result: test.php?test=replaced1&second_test=replaced1&third_test=value1

Affected versions: 1.4.11svn-20070602, 1.5.2svn-20070602