Using percent sign in assert message about simpletest HOT 8 CLOSED

Hey! I will try to reproduce the issue. Thank you for reporting.

Referencing:
https://github.com/simpletest/simpletest/blob/master/test_case.php#L311

from simpletest.

Can not reproduce. This is my code:

<?php

require_once dirname(__FILE__) . '/../autorun.php';
require_once dirname(__FILE__) . '/../test_case.php';

/**
 * @link https://github.com/simpletest/simpletest/issues/29
 */
class Issue29 extends UnitTestCase
{
    public function testPercentSignInMessage1()
    {
        $expectation = new EqualExpectation(100, '%');
        $this->assert($expectation, 100, '%');
    }

    public function testPercentSignInMessage2()
    {
        $expectation = new EqualExpectation(100, 'My expectation message: %s');
        $this->assert($expectation, 100, 'My assert message : %s');
    }

    public function testPercentSignInMessage3()
    {
        $expectation = new EqualExpectation(100, '%%');
        $this->assert($expectation, 100, '%%');
    }
}

test_issue29.php
OK
Test cases run: 1/1, Passes: 3, Failures: 0, Exceptions: 0

Please post an example, which allows the reproduction of this issue.

Thanks, Jens

from simpletest.

Thanx for quick response.
Please add method belowto your test case.
Looks like it's not simple single percent char issue. I narrowed it to a string in the example. Usually I'm facing it while adding URLs to message. Urls which contain two spaces (%20 sequence). For example:
http://www.domain.com/some%20long%20name.html
Surprisingly it passes while there is single space in url, like:
http://www.domain.com/some%20name.html

-- this test case throws "Too few arguments" exception
public function testPercentSignInMessage4()
{
    $this->assertEqual(1,0, '%2f%');
}

BTW: It throws exception even if assert passes. But it's expected since sprintf is used in both cases: for pass and fail (in assert() method of SimpleTestCase)

from simpletest.

Ok, thank you for the test case and explanation. I was able to reproduce it.

Right, the error case is this one:

$this->assertEqual(1,1, 'http://www.domain.com/some%20long%20name.html');

Usually I'm facing it while adding URLs to message. Urls which contain two spaces (%20 sequence).

The basic workaround would be using either urldecode() or rawurldecode()
on the URLs to turn them into a decoded form, so that it works with sprintf().

Surprisingly it passes while there is single space in url, like:

There is one message passed as second argument. So the number of sprintf arguments is satisfied in this case.

var_dump(sprintf('http://www.domain.com/some%20name.html', ''));

But, when you have multiple unescaped percentages, it will throw the "too few arguments" starting from the third argument (missing):
var_dump(sprintf('http://www.domain.com/some%20long%20name.html', '', X));

But it's expected since sprintf is used in both cases

Yes, that's right.

For now, my suggestion is to decode the URLs.
Because i'm lacking an idea on how to tackle this issue.
Should we escape or decode the percentages as a "bugfix"?
We could probably catch the error and enrich the error message,
to inform the user to escape the message beforehand.
Not sure how to handle this, yet.

from simpletest.

Seems like another way is to "escape" percent character by doubling it.
But I'm not sure it couldn't cause unexpected side-effects

from simpletest.

Seems like another way is to "escape" percent character by doubling it.

I agree and implemented it that way.

But I'm not sure it couldn't cause unexpected side-effects

Hmm, let's find out. Hopefully the tests cover us :)

Thank you for contributing and making SimpleTest better ❤

from simpletest.

I found that similar issue might be found in other parts of code.
In particular, I found it in methods WebTester::assertResponse and WebTester::assertMime

with regards

from simpletest.

Good catch.
Both functions pass $message to assertTrue() and we could add an additional call to the escaping function there.

from simpletest.