assert icon indicating copy to clipboard operation
assert copied to clipboard

Multi-layer encoding

Open theofidry opened this issue 2 years ago • 2 comments

When calling assert like so:

Assert::count(..., message: $message);

$message is forwarded to sprintf. As a result if you are to do something like this:

Assert::count(
    ...,
    message: 'This is my custom message %s', // <- the placeholder will be filled up automatically, great!
);

Which does mean if you are using your own sprintf you should escape it first:

Assert::count(
    ...,
    message: 'This is my %%w custom message %s', // <- manually escape "%w" as this would make sprintf fail (it is an invalid format specifier)
);

// Automatic escaping:

Assert::count(
    ...,
    message: str_replace('%', '%%', 'This is my %%w custom message %s'),
);

So far so good. Now the problem: this does not work for all assertions. Indeed the example above with Assert::count() will still result in ValueError: Unknown format specifier "w".

The root of the issue is that an assertion can rely on other assertions internally, e.g. ::count() calls ::eq(). So the custom escaped message becomes unescaped when passing it to ::eq(). Making it work requires the user to find out how many items the string is evaluated to escape it accordingly (in this example twice).

I think this is a design flaw and the way it could be fixed is to escape the message passed within Assert when relying on inner assertions:

// Assert.php

public static function count($array, $number, $message = '')
    {
        static::eq(
            \count($array),
            $number,
            \sprintf(
                self::sprintfEscape($message) ?: 'Expected an array to contain %d elements. Got: %d.',
                $number,
                \count($array)
            )
        );
    }
    
    public static function sprintfEscape(string $value): string {
      return str_replace('%', '%%', $value);
    }

theofidry avatar Oct 12 '23 10:10 theofidry

Will need to think about this one some more. I use a variety of custom messages, but generally I format the custom message before passing it to Assert::foo.

Maybe #221 would be a better solution to this?

shadowhand avatar Oct 15 '24 14:10 shadowhand

@shadowhand I think those are different problems entirely, but I am certainly in support of #221.

generally I format the custom message before passing it to Assert::foo

I do as well from time to time, but I don't like to figure out the "make mixed into a human readable string" logic which the library already does internally. So leveraging the placeholder is a nice feature

theofidry avatar Oct 15 '24 19:10 theofidry