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

Fix subsequent reads from php://input in ServerRequest #247

Merged
merged 1 commit into from Feb 20, 2019

Conversation

sagikazarmark
Copy link
Member

@sagikazarmark sagikazarmark commented Dec 7, 2018

According to the PHP documentation reading from the same php://input
stream is not possible. Even reading from separate streams is
SAPI dependent.

http://php.net/manual/de/wrappers.php.php#wrappers.php.input

This change adds a CachingStream wrapping the default stream
when using ServerRequest::fromGlobals to allow subsequent
reads.

Fixes #186

According to the PHP documentation reading from the same php://input
stream is not possible. Even reading from separate streams is
SAPI dependent.

http://php.net/manual/de/wrappers.php.php#wrappers.php.input

This change adds a CachingStream wrapping the default stream
when using ServerRequest::fromGlobals to allow subsequent
reads.
@sagikazarmark sagikazarmark requested review from Tobion and Nyholm Dec 7, 2018
@sagikazarmark sagikazarmark added this to the 1.6.0 milestone Dec 7, 2018
@sagikazarmark
Copy link
Member Author

@sagikazarmark sagikazarmark commented Dec 21, 2018

@Tobion @Nyholm can I get your opinion on this one?

@sagikazarmark
Copy link
Member Author

@sagikazarmark sagikazarmark commented Feb 20, 2019

Ping @Tobion @Nyholm

Nyholm
Nyholm approved these changes Feb 20, 2019
Copy link
Member

@Nyholm Nyholm left a comment

Sure, that input will never change. Why not cache it =)

@sagikazarmark sagikazarmark merged commit 31ea59d into master Feb 20, 2019
2 checks passed
@sagikazarmark sagikazarmark deleted the fix-input-stream-subsequent-read branch Feb 20, 2019
@Tobion
Copy link
Member

@Tobion Tobion commented Jun 6, 2019

Clever solution 👍 @sagikazarmark

LeSuisse added a commit to Enalean/tuleap that referenced this issue Feb 10, 2020
A change is made to the FileUploadController so the implementation
of the TusServer can continue to work. In guzzle/psr7#247 [0] the default
body of the ServerRequest has been switched to a CachingStream which is
nice until you need to use StreamInterface::detach() because only already
cached data will be available. Forcing data to be put in cache is not interesting
here because we want to save as much as data as possible from the client even
if we only got a part of it (resuming the upload will be dealt with the tus protocol).
To avoid that, we now recreate a StreamInterface directly from the php://input resource
when uploading a file with tus.

[0] guzzle/psr7#247

Change-Id: I167bac3958ffdec37d8b4cf8860d70d73befe3a1
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.

3 participants