Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Added stream rewind in JsonResponse constructor #115

Merged
merged 1 commit into from
Dec 16, 2015

Conversation

NikolaySl
Copy link

No description provided.

@weierophinney
Copy link
Member

Thanks for reporting this and providing a patch! I was about to merge, and realized the issue was also present in the HTML and text response types! I'm updating those as part of the merge.

@weierophinney weierophinney merged commit ffb22ed into zendframework:master Dec 16, 2015
weierophinney added a commit that referenced this pull request Dec 16, 2015
Added stream rewind in JsonResponse constructor
weierophinney added a commit that referenced this pull request Dec 16, 2015
The same issue observed in the `JsonResponse` was also present in the
`HtmlResponse` and `TextResponse`: the constructor was not rewinding the
body after creation, meaning `getContents()` was starting from the end
of the stream. Each now has tests for this behavior, and the constructor
logic now rewinds the body stream after creation.
weierophinney added a commit that referenced this pull request Dec 16, 2015
weierophinney added a commit that referenced this pull request Dec 16, 2015
weierophinney added a commit that referenced this pull request Dec 16, 2015
@NikolaySl NikolaySl deleted the Stream_rewind_issue branch December 16, 2015 16:31
@NikolaySl
Copy link
Author

Thanks for very fast fixes.

@weierophinney When new version will be released?

@weierophinney
Copy link
Member

@NikolaySl Already done; 1.3.1 has the fixes.

@mnapoli
Copy link
Contributor

mnapoli commented Jul 26, 2016

@weierophinney how are we supposed to write to the stream afterwards since it has been rewinded?

Let's say we have a middleware that adds additional content to the stream, $response->getBody()->write() will overwrite existing data in the stream?

@mnapoli
Copy link
Contributor

mnapoli commented Aug 17, 2016

Moved my question to #195, it affects all response classes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants