This repository has been archived by the owner on Jan 29, 2020. It is now read-only.
#311 - Update NormalizeUploadedFiles to handle multi-dimension recursively #312
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ocramius
reviewed
Jul 9, 2018
| @@ -24,6 +24,45 @@ | |||
| */ | |||
| function normalizeUploadedFiles(array $files) | |||
| { | |||
| /** | |||
| * @param array $tmpNameTree | |||
There was a problem hiding this comment.
Could you please document the types of these arrays?
There was a problem hiding this comment.
Will do 👍
EDIT Mis-remembered, have pushed commit to address the new function.
precariouspanther
force-pushed
the
311-recursive-normalize-files
branch
from
86da1ef
to
790f624
Compare
weierophinney
suggested changes
Jul 9, 2018
| $tmpNameTree[$key], | ||
| $sizeTree[$key], | ||
| $errorTree[$key], | ||
| $nameTree[$key] ?? null, |
There was a problem hiding this comment.
We're currently still supporting PHP 5.6, so this needs to become:
isset($nameTree[$key]) ? $nameTree[$key] : null(Here and everywhere else you use null coalesce.)
…ension recursively
Since we support PHP 5.6 still in this library, we cannot yet use null coalesce.
weierophinney
force-pushed
the
311-recursive-normalize-files
branch
from
790f624
to
1172baf
Compare
Describe what scenario is being tested!
weierophinney
added a commit
that referenced
this pull request
Jul 9, 2018
weierophinney
added a commit
that referenced
this pull request
Jul 9, 2018
Forward port #312 Conflicts: CHANGELOG.md
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To address #311 (Some/All?) SAPI's index the
tmp_nameetc common fields from the point that a numerically indexed array is encountered which is causing Diactoros to fail fatally with form uploads that use multi-dimensional named fields containing a numeric index (as well as not converting all leaf nodes for deeper hierarchies of file upload field names).This change updates normalizeUploadedFiles to be recursive and introduces several tests to verify the behaviour matches the PSR-7 spec + the scenario not mentioned by the spec.
Note: given the number of "private" functions being encapsulated in this larger function it may be worth considering converting this to an object instead especially as the recursion and lack of hoisting makes declaration order of the "private" functions non-idiomatic for most PHP engineers. I avoided significant changes here given it's just a fix, but food for thought!
Also, first time I've contributed to this project: please let me know if I've incorrectly interpreted the contrib. standards and I need to fix anything 😃
@PascalZajac @serroba FYI