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

Wrong type for $_FILES when uploading multiple files #8760

Open
kkmuffme opened this issue Nov 25, 2022 · 6 comments
Open

Wrong type for $_FILES when uploading multiple files #8760

kkmuffme opened this issue Nov 25, 2022 · 6 comments

Comments

@kkmuffme
Copy link
Contributor

As per last change in #8621 the type of $_FILES is wrong when using "multiple" attribute.

https://psalm.dev/r/be6a290e56

<form action="" method="post" enctype="multipart/form-data">
   <input type="file" name="images[]" multiple>
   <br>
   <input type="submit" value="Upload" name="submit">
</form>

$_FILES will contain an array of strings for each key, not a string in that case.

@danog

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/be6a290e56
<?php

$x = $_FILES;

/** @psalm-trace $_FILES */;
Psalm output (using commit 255a152):

INFO: Trace - 5:28 - $_FILES: array<non-empty-string, array{error: int<0, 8>, full_path: string, name: string, size: int<0, max>, tmp_name: string, type: string}>

INFO: UnusedVariable - 3:1 - $x is never referenced or the value is not used

@danog
Copy link
Collaborator

danog commented Nov 25, 2022

Even worse, $_FILES can contain arrays of arrays of arrays (infinitely nested arrays!) of strings, as per #8621.

The only way to fix this properly is to introduced specialized logic in the ArrayFetchAnalyzer.

@kkmuffme
Copy link
Contributor Author

Which I mostly added in #8473 and #8561 - but then these types are crap, so people complained and so you (kind of) reverted that in v5.

Any suggestions how we handle this best? Atm it's a complete pain to work with $_FILES as in 2/3 of cases I encounter it's wrong.

@danog
Copy link
Collaborator

danog commented Nov 25, 2022

As a hotfix, I'd recommend creating a wrapper function with a set of variadic parameters that returns a properly typed array, psalm-suppressing issues just for that function.

I gave a go at implementing the ArrayFetch logic when I removed your types, but it's messier than I thought it would be: I'll try to give it a go later, you can also try digging in that direction if you want.

@kkmuffme
Copy link
Contributor Author

kkmuffme commented Dec 2, 2022

@danog any further thoughts on this yet?

I'm not really sure what/how you intend to resolve this issue for all possibilities which are:

array<never, never> (= array{})
array{error: int<0, 8>, name: string, size: int<0, max>, tmp_name: string, type: string}
array{error: non-empty-list<int<0, 8>>, name: non-empty-list<string>, size: non-empty-list<int<0, max>>, tmp_name: non-empty-list<string>, type: non-empty-list<string>}
array<string, array{error: int<0, 8>, name: string, size: int<0, max>, tmp_name: string, type: string}>
array<string, array{error: non-empty-list<int<0, 8>>, name: non-empty-list<string>, size: non-empty-list<int<0, max>>, tmp_name: non-empty-list<string>, type: non-empty-list<string>}>

Which actually isn't correct too, since you can have a mix of them too if 1 input has multiple attribute but others do not, like e.g.

array<string, array{error: int<0, 8>, name: string, size: int<0, max>, tmp_name: string, type: string}|array{error: non-empty-list<int<0, 8>>, name: non-empty-list<string>, size: non-empty-list<int<0, max>>, tmp_name: non-empty-list<string>, type: non-empty-list<string>}>

@danog
Copy link
Collaborator

danog commented Dec 2, 2022

There is no way of solving this with a specific type, the only way is with a "hack" that directly modifies the array key resolution logic to always return a array{error: int<0, 8>, name: string, size: int<0, max>, tmp_name: string, type: string} when using $_FILES['...'][$last], $_FILES['...']['...'][$last], $_FILES['...']['...']['...'][$last] and so on where $last is 'error', 'name', 'size', 'tmp_name', 'type'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants