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 added this to the 1.6.0 milestone Dec 7, 2018
@sagikazarmark
Copy link
Member Author

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

@sagikazarmark
Copy link
Member Author

Ping @Tobion @Nyholm

Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@sagikazarmark sagikazarmark merged commit 31ea59d into master Feb 20, 2019
@sagikazarmark sagikazarmark deleted the fix-input-stream-subsequent-read branch February 20, 2019 09:30
@Tobion
Copy link
Member

Tobion commented Jun 6, 2019

Clever solution 👍 @sagikazarmark

LeSuisse added a commit to Enalean/tuleap that referenced this pull request 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.

Can't read body stream from ServerRequest::fromGlobals() twice.
3 participants