Skip to content

Validate sprintf argument types against format specifiers #13496

@angeleg

Description

@angeleg

Feature request

Problem

The sprintf function takes arguments and coerces them into a certain type based on the format specifier that is used (%d, %s, etc.).
See docs for the full list of all the possible specifiers: https://www.php.net/manual/en/function.sprintf.php

This comes with all the issues we know with type coercing...

For instance, accidentally using %d for 0e0903d0-42c9-4083-857d-047f6d779ae8 will make it 0.

Solution

We could have phpstan check that the argument type and the specifier match.

Here is a custom rule we made, as a proof of concept:

/**
 * @implements Rule<FuncCall>
 */
final class SprintfFormatSpecifierRule implements Rule
{
    #[Override]
    public function getNodeType() : string
    {
        return FuncCall::class;
    }

    #[Override]
    public function processNode(Node $node, Scope $scope) : array
    {
        if ( ! $node->name instanceof Name) {
            return [];
        }

        $functionName = $node->name->toString();

        if ($functionName !== 'sprintf') {
            return [];
        }

        $args = $node->getArgs();

        if (count($args) < 1) {
            return [];
        }

        $formatStringType = $scope->getType($args[0]->value);

        $errors = [];

        foreach ($formatStringType->getConstantStrings() as $constantString) {
            $formatString = $constantString->getValue();

            $specifiers = $this->extractFormatSpecifiers($formatString);

            foreach ($specifiers as $index => $specifier) {
                $argIndex = $index + 1;

                if ( ! isset($args[$argIndex])) {
                    continue;
                }

                $argumentType = $scope->getType($args[$argIndex]->value);
                $error = $this->validateSpecifierAgainstType($specifier, $argumentType, $argIndex);

                if ($error !== null) {
                    $errors[] = $error;
                }
            }
        }

        return $errors;
    }

    /**
     * @return list<string>
     */
    private function extractFormatSpecifiers(string $formatString) : array
    {
        $specifiers = [];
        $matches = [];

        if (preg_match_all('/%(?:\d+\$)?(?:[+\- 0#]*|\'.)*(?:\d+|\*)?(?:\.(?:\d+|\*))?([bcdeEfFgGhHosuxX])/', $formatString, $matches) > 0) {
            $specifiers = $matches[1];
        }

        return $specifiers;
    }

    private function validateSpecifierAgainstType(string $specifier, Type $argumentType, int $argIndex) : ?IdentifierRuleError
    {
        switch ($specifier) {
            case 's':
                $stringType = new StringType();
                $stringableType = new ObjectType(Stringable::class);
                $validType = new UnionType([$stringType, $stringableType]);

                if ( ! $validType->isSuperTypeOf($argumentType)->yes()) {
                    return RuleErrorBuilder::message(
                        'sprintf() format specifier %' . $specifier . ' at position ' . $argIndex . ' expects string or Stringable, ' . $argumentType->describe(VerbosityLevel::typeOnly()) . ' given.',
                    )->identifier('ticketswap.sprintf.invalidFormatSpecifier')->build();
                }

                break;

            case 'd':
            case 'u':
            case 'c':
            case 'o':
            case 'x':
            case 'X':
            case 'b':
                $intType = new IntegerType();

                if ( ! $intType->isSuperTypeOf($argumentType)->yes()) {
                    return RuleErrorBuilder::message(
                        'sprintf() format specifier %' . $specifier . ' at position ' . $argIndex . ' expects int, ' . $argumentType->describe(VerbosityLevel::typeOnly()) . ' given.',
                    )->identifier('ticketswap.sprintf.invalidFormatSpecifier')->build();
                }

                break;

            case 'f':
            case 'F':
            case 'g':
            case 'G':
            case 'h':
            case 'H':
                $floatType = new FloatType();

                if ( ! $floatType->isSuperTypeOf($argumentType)->yes()) {
                    return RuleErrorBuilder::message(
                        'sprintf() format specifier %' . $specifier . ' at position ' . $argIndex . ' expects float, ' . $argumentType->describe(VerbosityLevel::typeOnly()) . ' given.',
                    )->identifier('ticketswap.sprintf.invalidFormatSpecifier')->build();
                }

                break;
        }

        return null;
    }
}

Curious to hear what everyone thinks? 🙂

Did PHPStan help you today? Did it make you happy in any way?

Yes! We love PHPStan at TicketSwap 🩵

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions