Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[11.x] Fix client path value in file uploads #53941

Merged
merged 3 commits into from
Dec 17, 2024

Conversation

gyaaniguy
Copy link
Contributor

@gyaaniguy gyaaniguy commented Dec 17, 2024

PHP 8.1 introduced the 'full_path' key in $_FILES during file uploads. The purpose is to support Folder uploads.

array(1) {
  ["files"]=>
  array(6) {
    ["name"]=>
    array(2) {
      [0]=>
      string(23) "laravel-svgrepo-com.svg"
    }
    ["full_path"]=>
    array(2) {
      [0]=>
      string(30) "foldit/laravel-svgrepo-com.svg"
    }
  }
}

Symphony handles this. When a new instance of UploadedFile is created, $file['full_path'] ?? $file['name'] is passed to it
originalPath is set to the full value -> /path/filename.txt -> getClientOriginalPath()
originalName is set to the file name after extracting it from the full_path (if needed) // filename.txt

    public function __construct(string $path, string $originalName, ?string $mimeType = null, ?int $error = null, bool $test = false)
    {
        $this->originalName = $this->getName($originalName);
        $this->originalPath = strtr($originalName, '\\', '/');
$file = new UploadedFile($file['tmp_name'], $file['full_path'] ?? $file['name'], $file['type'], $file['error'], false);

Laravel doesn't handle this change.

InteractsWithInput.php file:
convertUploadedFiles() is defined. "Convert the given array of Symfony UploadedFiles to custom Laravel UploadedFiles."

convertUploadedFiles calls UploadedFile::createFromBase($file). This is where the Issue lies (UploadedFile.php ln 144)

A new instance of uploadedFile is created using the 'getClientOriginalName' causing us to lose the 'full_path' value. Which was stored in getClientOriginalPath()

  public static function createFromBase(SymfonyUploadedFile $file, $test = false)
    {
        return $file instanceof static ? $file : new static(
            $file->getPathname(),
            $file->getClientOriginalName(), //problem
            $file->getClientMimeType(),
            $file->getError(),
            $test
        );
    }

Note: path here is not the temporary path. Not the uploadedTo path. But the original path sent by the client in the full_path key.

My fix: is to use getClientOriginalPath() instead. I think it should be okay. Since symfony extracts the originalname from the full_path.

This is my first attempt of such a PR. Please do tell if I have missed anything.

Thank you!

@gyaaniguy gyaaniguy marked this pull request as draft December 17, 2024 10:10
@gyaaniguy
Copy link
Contributor Author

gyaaniguy commented Dec 17, 2024

hmm it seems my changes are dependent on http-foundation : 7.2.0 .
Whereas the tests lock the version to 7.0.3
Locking symfony/http-foundation (v7.0.3)

I am gonna try updating composer.json. Not sure if I should though..

Edit: well updating composer and fixing styling, seems to have fixed the tests as well.

@gyaaniguy gyaaniguy marked this pull request as ready for review December 17, 2024 11:09
@taylorotwell taylorotwell merged commit 4526005 into laravel:11.x Dec 17, 2024
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants