This repository has been archived by the owner on Jan 31, 2020. It is now read-only.
PSR-7 Uploaded File compatibility #237
Merged
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
2 tasks
|
@alextech please rebase against develop branch to only show your commit: If you found conflict, you can fix it in the file conflicted, and run: Lastly, force push: |
b86ac61 to
b74bd08
Compare
|
@samsonasik Thank you for instructions! I saved them to my notes. |
3211eb5 to
2cfda42
Compare
src/File/Upload.php
Outdated
| $content->getStream()->getMetadata('uri') : | ||
| $content['tmp_name']; | ||
|
|
||
| if (! is_uploaded_file($tmpFile)) { |
There was a problem hiding this comment.
The above logic is incorrect.
For PSR-7, we can assume that if we have UPLOAD_ERROR_OK, then the upload performed correctly. Why? because tmp_name has been taken out of the equation, and we're working with streams instead.
I'll update this during merge.
test/File/UploadFileTest.php
Outdated
| @@ -10,6 +10,7 @@ | |||
| namespace ZendTest\Validator\File; | |||
|
|
|||
| use PHPUnit\Framework\TestCase; | |||
| use Zend\Diactoros\UploadedFile; | |||
There was a problem hiding this comment.
I will update the patch to use PSR-7 mocks instead.
test/File/UploadTest.php
Outdated
| * | ||
| * @return void | ||
| */ | ||
| public function testPsrBasic() |
There was a problem hiding this comment.
I will update this method to instead use a data provider, with mock uploads.
- Removes zend-diactoros from requirements.
- Updates tests to use mock `UploadedFileInterface` instances, instead
of using zend-diactoros; this keeps the support agnostic.
- Modifies `UploadFile` to store the `UploadedFileInterface` instance
instead of the stream URI; this is more consistent with the
implementation of `Upload`, which stores an array of
`UploadedFileInterface` instances.
- The detection of an attack is moot with PSR-7, as it stores the
uploaded file as an in-memory stream. As such, I modified both classes
to omit that check; if we get an `UPLOAD_ERR_OK`, we have a valid
upload at that point.
- I extracted three methods from `UploadFile::isValid()`:
- `validateFileFromErrorCode()` does a switch around the error code,
setting errors and returning a boolean.
- `validateUploadedFile()` is used in both the string and array value
cases of `isValid()` to validate the incoming file. If
`UPLOAD_ERR_OK` is detected, it performs additional logic to detect
an upload attack, but otherwise delegates to
`validateFileFromErrorCode()`.
- `validatePsr7UploadedFile()` is used when the value is an
`UploadedFileInterface`, and delegates to
`validateFileFromErrorCode()`, using the result of `getError()`.
- I refactored the new unit tests to use data providers whenever
possible.
2cfda42 to
a10270a
Compare
|
Thanks, @alextech! |
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.
This patch provides changes to both the
File\UploadandFile\UploadFilevalidators to provide support for validating PSR-7UploadedFileInterfaceinstances.