Do not rewind streams

[Nyholm]

Should we rewind streams and resources when they are passed in the factory?

Guzzle does not rewind at all.
Slim rewinds resources but not streams.
Zend rewinds everything.

This is related to #71

[dbu]

if we don't read from the stream, we also should not rewind it. i am a bit afraid about the side effect if we change behaviour within a minor version. we should probably only do that for version 2 and add notes to the corresponding changelogs.

[Nyholm]

I think we can update this behavior in a patch release. The factories are not interchangeable, which is a bug. What I want to know is: What is our intent?

I agree that we should not touch an existing StreamInterface.

Why do we like to rewind it? What benefits does it give?

[joelwurtz]

Should we rewind streams and resources when they are passed in the factory?

IMO no

We should only rewind when we read something and we know that another piece of code will certainly need to read it to.

[Nyholm]

Since we do not have to rewind the stream I could reduce the complexity and increase readability of the factories.

[Nyholm]

Thank you for the feedback. Is this PR ready to be merged?
This PR is blocking: php-http/multipart-stream-builder#27

[sagikazarmark]

How do http clients behave when they receive a request? Do they rewind the body?

What is our expectation (and current behaviour) in plugins (where we interact with streams the most apart from the client)?

I guess these need to be taken into account as well. Maybe we could test this branch in our integration test suite with different clients.

[Nyholm]

How do http clients behave when they receive a request? Do they rewind the body?

php-http/curl-client: Does not rewind
php-http/socket-client: Does rewind
Guzzle: Uses __toString() which rewinds

What is our expectation (and current behaviour) in plugins (where we interact with streams the most apart from the client)?

Plugins that handles responses do always rewind the stream. The end user should not get different responses depending on if a plugin (say LogPlugin) is used or not.

---

I believe it is the client's responsibility to rewind the stream. Why would you ever want to create a stream, seek to 50% and then just send the second half as a HTTP request?

I will send a PR to php-http/curl-client

[dbu]

now with your fix, curl-client does rewind as well

[Nyholm]

Yes, Correct. @sagikazarmark do you want to investigate some more before we merge? Do you think it is a good idea to merge?

[sagikazarmark]

No, if the consensous is that clients mostly rewind, I am fine with that, users probably expect that behaviour anyway.

Also, stream factories will probably never produce a non-seekable stream.

There is one risk though: one might pass in a stream which is read only, non-seekable. How do we handle that case?

[Nyholm]

There is one risk though: one might pass in a stream which is read only, non-seekable. How do we handle that case?

We do not touch streams at all. We just return them. Same with resources. We do not read form them at all, just wrap them in a stream.

[sagikazarmark]

Okay, so I am totally confused now I guess. 😄 Isn't this PR about REWINDING streams? Or about NOT REWINDING them?

[Nyholm]

Sorry for the confusion. I updated the code after #72 (comment) but I did not update the title.

[sagikazarmark]

Ah, okay. 😄 I was on the wrong path then for a long time ago.

[sagikazarmark]

Good to go?

[Nyholm]

Yes!