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

multiple File upload issue #679

Closed
Iamscalla opened this issue Aug 16, 2017 · 20 comments
Closed

multiple File upload issue #679

Iamscalla opened this issue Aug 16, 2017 · 20 comments

Comments

@Iamscalla
Copy link
Contributor

upload of multiple files returns null

below is what i tried
<input accept="image/*" multiple type="file" id="feed-editor-image-input" name="images[]"/>
$imagesFile = $this->request->getFiles('images'); var_dump($imagesFile);

@JakeAi
Copy link
Contributor

JakeAi commented Aug 16, 2017

@chistel In the function getFiles() on line 487 of the IncomingRequest class, if you change

public function getFiles(): FileCollection
{
	if (is_null($this->files))
	{
		$this->files = new FileCollection();
	}
	return $this->files;
}

to

public function getFiles()
{
	if (is_null($this->files))
	{
		$this->files = new FileCollection();
	}
	return $this->files->all();
}

It will work for now. I would let @lonnieezell check over this though because it is a dead end with getFiles().

@lonnieezell
Copy link
Member

A couple of thoughts for you @chistel:

  1. Is your form setup with enctype=multipart/form-data? Forgetting that is a common error, for me anyway.
  2. Did you dump the $_FILES variable to ensure there was anything actually uploaded that the server recognized?
  3. You're calling getFiles('images), however that method simply returns all files within a FileCollection class (and, yes, @JakeAi that's the way it should be. The FileCollection class provides some handy helper functions you might want to use.). To retrieve a single file, you should use the getFile() method (singular). Or use getFiles() and then you can do checks like hasFile(), etc.
  4. Finally, when trying to access a single file of an array of files like you have here, you should use dot array syntax.

@Iamscalla
Copy link
Contributor Author

@JakeAi That was my first solution, but it returned error say Hercules must be an instance of FileCollection except I have to remove the instance from it

@Iamscalla
Copy link
Contributor Author

@lonnieezell pls can you give a sample of running a multi file upload to a folder using the getFiles() method

@lonnieezell
Copy link
Member

@chistel I might be able to pull something together tonight (just getting my morning started here), but you never answered the questions - especially #1 and #2.

@Iamscalla
Copy link
Contributor Author

Iamscalla commented Aug 16, 2017

@lonnieezell Oh skip me, #1 & #2 did that, and 3 I figured that I was on the wrong path.
Now it's returns all. but it would be better using specific field name than just returning all uploaded file. Especially if I have two different file field in a single form

@JakeAi
Copy link
Contributor

JakeAi commented Aug 16, 2017

@lonnieezell I couldn't even get getFiles to work without modifications. getFiles creates a new FileCollection class, but there is no construct and doesn't call any functions?, and this->files is null. If you call getFile('w/e') and then getFiles, it works.

 <form action="/Shared/Test" method="post" enctype="multipart/form-data">
        <input multiple type="file" id="feed-editor-image-input" name="images[]"/>
        <input type="submit">
    </form>
// This doesn't work
	print_r( [
			$_FILES,
			$this->request->getFiles(),
		] );
Array
(
    [0] => Array
        (
            [images] => Array
                (
                    [name] => Array
                        (
                            [0] => 170586-Subaru_Impreza_WRX-Subaru_Impreza-Subaru-JDM.png
                            [1] => 17499755_1403719142999697_1454939783_o.jpg
                        )

                    [type] => Array
                        (
                            [0] => 
                            [1] => image/jpeg
                        )

                    [tmp_name] => Array
                        (
                            [0] => 
                            [1] => D:\Wamp\tmp\php6315.tmp
                        )

                    [error] => Array
                        (
                            [0] => 1
                            [1] => 0
                        )

                    [size] => Array
                        (
                            [0] => 0
                            [1] => 118352
                        )

                )

        )

    [1] => CodeIgniter\HTTP\Files\FileCollection Object
        (
            [files:protected] => 
        )

)
// This works
	print_r( [
			$_FILES,
			$this->request->getFile( 'images' ),
			$this->request->getFiles(),
		] );
Array
(
    [0] => Array
        (
            [images] => Array
                (
                    [name] => Array
                        (
                            [0] => 170586-Subaru_Impreza_WRX-Subaru_Impreza-Subaru-JDM.png
                            [1] => 17499755_1403719142999697_1454939783_o.jpg
                        )

                    [type] => Array
                        (
                            [0] => 
                            [1] => image/jpeg
                        )

                    [tmp_name] => Array
                        (
                            [0] => 
                            [1] => D:\Wamp\tmp\php6141.tmp
                        )

                    [error] => Array
                        (
                            [0] => 1
                            [1] => 0
                        )

                    [size] => Array
                        (
                            [0] => 0
                            [1] => 118352
                        )

                )

        )

    [1] => 
    [2] => CodeIgniter\HTTP\Files\FileCollection Object
        (
            [files:protected] => Array
                (
                    [images] => Array
                        (
                            [0] => CodeIgniter\HTTP\Files\UploadedFile Object
                                (
                                    [path:protected] => 
                                    [originalName:protected] => 170586-Subaru_Impreza_WRX-Subaru_Impreza-Subaru-JDM.png
                                    [name:protected] => 170586-Subaru_Impreza_WRX-Subaru_Impreza-Subaru-JDM.png
                                    [originalMimeType:protected] => 
                                    [error:protected] => 1
                                    [hasMoved:protected] => 
                                    [size:protected] => 0
                                    [pathName:SplFileInfo:private] => 
                                    [fileName:SplFileInfo:private] => 
                                )

                            [1] => CodeIgniter\HTTP\Files\UploadedFile Object
                                (
                                    [path:protected] => D:\Wamp\tmp\php6141.tmp
                                    [originalName:protected] => 17499755_1403719142999697_1454939783_o.jpg
                                    [name:protected] => 17499755_1403719142999697_1454939783_o.jpg
                                    [originalMimeType:protected] => image/jpeg
                                    [error:protected] => 0
                                    [hasMoved:protected] => 
                                    [size:protected] => 118352
                                    [pathName:SplFileInfo:private] => D:\Wamp\tmp\php6141.tmp
                                    [fileName:SplFileInfo:private] => php6141.tmp
                                )

                        )

                )

        )

)

@Iamscalla
Copy link
Contributor Author

@JakeAi I figured it out in to ways, either add the all method of FileCollection into a constructor or go with the previous method you suggested but removing the fileCollection instance from getFiles() method

@Iamscalla
Copy link
Contributor Author

@lonnieezell Never mind about my last response and request, I figured it out. I would make a doc regarding it.

@JakeAi
Copy link
Contributor

JakeAi commented Aug 16, 2017

@chistel All you need to do is return all() and remove the return type after getFiles(). You don't need to necessarily remove the instance.

public function getFiles(): FileCollection // <-----
{
	if (is_null($this->files))
	{
		$this->files = new FileCollection();
	}
	return $this->files; // <-----
}

to

public function getFiles() // <-----
{
	if (is_null($this->files))
	{
		$this->files = new FileCollection();
	}
	return $this->files->all(); // <-----
}

@Iamscalla
Copy link
Contributor Author

@JakeAi exactly what I did

@JakeAi
Copy link
Contributor

JakeAi commented Aug 16, 2017

@chistel Oh, right on. I'm still confused though. Is @lonnieezell right? Or is something broken? Like, I understand about the helper functions, but I couldn't get it to work at all. populateFiles doesn't get called at all through getFiles(). Are you supposed to $files=getFiles() then $files->getFile()? I feel like it's an inconvenience. I'd like to get files and then foreach the files and call the helpers on those iterations?

@Iamscalla
Copy link
Contributor Author

@JakeAi he is right, I can't find anything broken there.. when I make the doc, I would clarify it

@JakeAi
Copy link
Contributor

JakeAi commented Aug 16, 2017

@chistel Can you post your sample code?

@Iamscalla
Copy link
Contributor Author

Iamscalla commented Aug 16, 2017

@JakeAi am on mobile now, but here is what I have,

If($imagefile = $this->request->getFiles())
{  
   foreach($imagefile['images'] as $Im)
   {
      if ($im->isValid() && ! $im->hasMoved())
      {
           $newName = $img->getRandomName();
           $img->move(WRITEPATH.'uploads', $newName);
      }
   }
}

where the images is the form field name
<input type="file" name="images[]" multiple />

@JakeAi
Copy link
Contributor

JakeAi commented Aug 16, 2017

@chistel did you add the all() to the IncomingRequest then? Because your code doesn't work out of the box.

@Iamscalla
Copy link
Contributor Author

@JakeAi yes I added it

@JakeAi
Copy link
Contributor

JakeAi commented Aug 16, 2017

@chistel Ahhh, that's what I meant about being broken. So something does need changed.

@Iamscalla
Copy link
Contributor Author

@JakeAi 🙄🙄🙄 yes just that

@lonnieezell
Copy link
Member

@chistel 's changes have been merged in, so closing this.

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

No branches or pull requests

3 participants