zend-mvc icon indicating copy to clipboard operation
zend-mvc copied to clipboard

fixed SimpleStreamResponseSender ignored defined content length

Open marc-mabe opened this issue 6 years ago • 2 comments

Provide a narrative description of what you are trying to accomplish:

We are using the stream HTTP response to directly stream file content from S3 to clients. As we are dealing with video files we are using the the Range/Content-Range headers to allow clients to download specific parts of files.

To be able to do so we use the Zend\Http\Response\Stream::setContentLength method together with fseek to define the parts of the file to send.

Where Stream::readStream is dealing with the content length it's not handled in SimpleStreamResponseSender and the stream gets send until EOF.

To fix that this PR uses stream_copy_to_stream to php://output instead of fpassthru if a content length has been defined.

  • [x] Are you fixing a bug?

    • [x] Detail how the bug is invoked currently.
    • [x] Detail the original, incorrect behavior.
    • [x] Detail the new, expected behavior.
    • [x] Base your feature on the master branch, and submit against that branch.
    • [x] Add a regression test that demonstrates the bug, and proves the fix.
    • [ ] Add a CHANGELOG.md entry for the fix.
  • [ ] Are you creating a new feature?

    • [ ] Why is the new feature needed? What purpose does it serve?
    • [ ] How will users use the new feature?
    • [ ] Base your feature on the develop branch, and submit against that branch.
    • [ ] Add only one feature per pull request; split multiple features over multiple pull requests
    • [ ] Add tests for the new feature.
    • [ ] Add documentation for the new feature.
    • [ ] Add a CHANGELOG.md entry for the new feature.
  • [ ] Is this related to quality assurance?

  • [ ] Is this related to documentation?

marc-mabe avatar Sep 18 '19 17:09 marc-mabe

This repository has been closed and moved to laminas/laminas-mvc; a new issue has been opened at https://github.com/laminas/laminas-mvc/issues/2.

weierophinney avatar Dec 31 '19 21:12 weierophinney

This repository has been moved to laminas/laminas-mvc. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:

  • Squash all commits in your branch (git rebase -i origin/{branch})
  • Make a note of all changed files (`git diff --name-only origin/{branch}...HEAD
  • Run the laminas/laminas-migration tool on the code.
  • Clone laminas/laminas-mvc to another directory.
  • Copy the files from the second bullet point to the clone of laminas/laminas-mvc.
  • In your clone of laminas/laminas-mvc, commit the files, push to your fork, and open the new PR. We will be providing tooling via laminas/laminas-migration soon to help automate the process.

weierophinney avatar Dec 31 '19 21:12 weierophinney