Possible format string issue in Slim\Http\UploadedFile::moveTo()

[sjinks]

throw new RuntimeException(sprintf('Error moving uploaded file %1s to %2s', $this->name, $targetPath));
throw new RuntimeException(sprintf('Error removing uploaded file %1s', $this->name));
throw new RuntimeException(sprintf('%1s is not a valid uploaded file', $this->file));
throw new RuntimeException(sprintf('Error moving uploaded file %1s to %2s', $this->name, $targetPath));
throw new RuntimeException(sprintf('Error moving uploaded file %1s to %2s', $this->name, $targetPath));

According to the documentation, 1 and 2 in %1s and %2s are width specifiers (that is, the minimum length of the converted string).

That is, %1s will give the string with at least one character, %2s with at least to characters, that is:

> echo sprintf("Error moving uploaded file %1s to %2s", "a", "b");
Error moving uploaded file a to  b

But other than with empty or single letter file names, this is probably meaningless.

As far as I understand, the original intent was to facilitate internationalization to make sure that the first parameter is always the name of the source file, and the second one is the destination. If so, the correct syntax will be

sprintf('Error moving uploaded file %1$s to %2$s', $this->name, $targetPath)

E.g.,

> echo sprintf("Error moving uploaded file %1\$s to %2\$s\n", "file1", "file2");
Error moving uploaded file file1 to file2
> echo sprintf("Error moving uploaded file %2\$s to %1\$s\n", "file1", "file2");
Error moving uploaded file file2 to file1

[geggleto]

I don't see an actual problem here?

[llvdl]

It seems to be confusing. I think the author meant to use numbered placeholders, not padded strings. I propose just using plain placeholders (%s). I have added a PR.

[designermonkey]

As the PR was merged to alter behaviour, and no more info or comments have been added by @sjinks, I'm closing this issue. If there is any more info, please add it and it can be reopened if necessary.