-
-
Notifications
You must be signed in to change notification settings - Fork 2k
gh-1838 added checks for pipe streams #1872
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
Changes from 5 commits
1431b30
e60867c
6503787
89a18a3
0199b88
2e5a54b
3882722
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,11 @@ | |
| */ | ||
| class Stream implements StreamInterface | ||
| { | ||
| /** | ||
| * This is octal as per header stat.h | ||
| */ | ||
| const FSTAT_MODE_S_IFIFO = 0010000; | ||
|
|
||
| /** | ||
| * Resource modes | ||
| * | ||
|
|
@@ -72,6 +77,13 @@ class Stream implements StreamInterface | |
| */ | ||
| protected $size; | ||
|
|
||
| /** | ||
| * Is this stream a pipe? | ||
| * | ||
| * @var type | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a |
||
| */ | ||
| protected $pipe; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please name this property
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would collide with the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. PHP can tell the difference between a property and a method with the same name…
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't know that :) |
||
|
|
||
| /** | ||
| * Create a new Stream. | ||
| * | ||
|
|
@@ -158,6 +170,7 @@ public function detach() | |
| $this->writable = null; | ||
| $this->seekable = null; | ||
| $this->size = null; | ||
| $this->pipe = null; | ||
|
||
|
|
||
| return $oldResource; | ||
| } | ||
|
|
@@ -196,7 +209,11 @@ public function __toString() | |
| public function close() | ||
| { | ||
| if ($this->isAttached() === true) { | ||
| fclose($this->stream); | ||
| if ($this->isPipe()) { | ||
| pclose($this->stream); | ||
| } else { | ||
| fclose($this->stream); | ||
| } | ||
| } | ||
|
|
||
| $this->detach(); | ||
|
|
@@ -211,7 +228,7 @@ public function getSize() | |
| { | ||
| if (!$this->size && $this->isAttached() === true) { | ||
| $stats = fstat($this->stream); | ||
| $this->size = isset($stats['size']) ? $stats['size'] : null; | ||
| $this->size = isset($stats['size']) && !$this->isPipe() ? $stats['size'] : null; | ||
| } | ||
|
|
||
| return $this->size; | ||
|
|
@@ -226,7 +243,7 @@ public function getSize() | |
| */ | ||
| public function tell() | ||
| { | ||
| if (!$this->isAttached() || ($position = ftell($this->stream)) === false) { | ||
| if (!$this->isAttached() || ($position = ftell($this->stream)) === false || $this->isPipe()) { | ||
| throw new RuntimeException('Could not get the position of the pointer in stream'); | ||
| } | ||
|
|
||
|
|
@@ -251,13 +268,17 @@ public function eof() | |
| public function isReadable() | ||
| { | ||
| if ($this->readable === null) { | ||
| $this->readable = false; | ||
| if ($this->isAttached()) { | ||
| $meta = $this->getMetadata(); | ||
| foreach (self::$modes['readable'] as $mode) { | ||
| if (strpos($meta['mode'], $mode) === 0) { | ||
| $this->readable = true; | ||
| break; | ||
| if ($this->isPipe()) { | ||
| $this->readable = true; | ||
| } else { | ||
| $this->readable = false; | ||
| if ($this->isAttached()) { | ||
| $meta = $this->getMetadata(); | ||
| foreach (self::$modes['readable'] as $mode) { | ||
| if (strpos($meta['mode'], $mode) === 0) { | ||
| $this->readable = true; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -300,7 +321,7 @@ public function isSeekable() | |
| $this->seekable = false; | ||
| if ($this->isAttached()) { | ||
| $meta = $this->getMetadata(); | ||
| $this->seekable = $meta['seekable']; | ||
| $this->seekable = !$this->isPipe() && $meta['seekable']; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -406,4 +427,22 @@ public function getContents() | |
|
|
||
| return $contents; | ||
| } | ||
|
|
||
| /** | ||
| * Returns whether or not the stream is a pipe. | ||
| * | ||
| * @return bool | ||
| */ | ||
| public function isPipe() | ||
| { | ||
| if ($this->pipe === null) { | ||
| $this->pipe = false; | ||
| if ($this->isAttached()) { | ||
| $mode = fstat($this->stream)['mode']; | ||
| $this->pipe = ($mode & self::FSTAT_MODE_S_IFIFO) !== 0; | ||
| } | ||
| } | ||
|
|
||
| return $this->pipe; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,158 @@ | ||
| <?php | ||
| /** | ||
| * Slim Framework (http://slimframework.com) | ||
| * | ||
| * @link https://github.com/slimphp/Slim | ||
| * @copyright Copyright (c) 2011-2016 Josh Lockhart | ||
| * @license https://github.com/slimphp/Slim/blob/master/LICENSE.md (MIT License) | ||
| */ | ||
| namespace Slim\Tests\Http; | ||
|
|
||
| use Slim\Http\Stream; | ||
|
|
||
| class StreamTest extends \PHPUnit_Framework_TestCase | ||
| { | ||
| /** | ||
| * @var resource pipe stream file handle | ||
| */ | ||
| private $pipeFh; | ||
|
|
||
| /** | ||
| * @var Stream | ||
| */ | ||
| private $pipeStream; | ||
|
|
||
| public function tearDown() | ||
| { | ||
| if ($this->pipeFh != null) { | ||
| stream_get_contents($this->pipeFh); // prevent broken pipe error message | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @covers Slim\Http\Stream::isPipe | ||
| */ | ||
| public function testIsPipe() | ||
| { | ||
| $this->openPipeStream(); | ||
|
|
||
| $this->assertTrue($this->pipeStream->isPipe()); | ||
|
|
||
| $this->pipeStream->detach(); | ||
| $this->assertFalse($this->pipeStream->isPipe()); | ||
|
|
||
| $fhFile = fopen(__FILE__, 'r'); | ||
| $fileStream = new Stream($fhFile); | ||
| $this->assertFalse($fileStream->isPipe()); | ||
| } | ||
|
|
||
| /** | ||
| * @covers Slim\Http\Stream::isReadable | ||
| */ | ||
| public function testIsPipeReadable() | ||
| { | ||
| $this->openPipeStream(); | ||
|
|
||
| $this->assertTrue($this->pipeStream->isReadable()); | ||
| } | ||
|
|
||
| /** | ||
| * @covers Slim\Http\Stream::isSeekable | ||
| */ | ||
| public function testPipeIsNotSeekable() | ||
| { | ||
| $this->openPipeStream(); | ||
|
|
||
| $this->assertFalse($this->pipeStream->isSeekable()); | ||
| } | ||
|
|
||
| /** | ||
| * @covers Slim\Http\Stream::seek | ||
| * @expectedException \RuntimeException | ||
| */ | ||
| public function testCannotSeekPipe() | ||
| { | ||
| $this->openPipeStream(); | ||
|
|
||
| $this->pipeStream->seek(0); | ||
| } | ||
|
|
||
| /** | ||
| * @covers Slim\Http\Stream::tell | ||
| * @expectedException \RuntimeException | ||
| */ | ||
| public function testCannotTellPipe() | ||
| { | ||
| $this->openPipeStream(); | ||
|
|
||
| $this->pipeStream->tell(); | ||
| } | ||
|
|
||
| /** | ||
| * @covers Slim\Http\Stream::rewind | ||
| * @expectedException \RuntimeException | ||
| */ | ||
| public function testCannotRewindPipe() | ||
| { | ||
| $this->openPipeStream(); | ||
|
|
||
| $this->pipeStream->rewind(); | ||
| } | ||
|
|
||
| /** | ||
| * @covers Slim\Http\Stream::getSize | ||
| */ | ||
| public function testPipeGetSizeYieldsNull() | ||
| { | ||
| $this->openPipeStream(); | ||
|
|
||
| $this->assertNull($this->pipeStream->getSize()); | ||
| } | ||
|
|
||
| /** | ||
| * @covers Slim\Http\Stream::close | ||
| */ | ||
| public function testClosePipe() | ||
| { | ||
| $this->openPipeStream(); | ||
|
|
||
| stream_get_contents($this->pipeFh); // prevent broken pipe error message | ||
| $this->pipeStream->close(); | ||
| $this->pipeFh = null; | ||
|
|
||
| $this->assertFalse($this->pipeStream->isPipe()); | ||
| } | ||
|
|
||
| /** | ||
| * @covers Slim\Http\Stream::__toString | ||
| */ | ||
| public function testPipeToString() | ||
| { | ||
| $this->openPipeStream(); | ||
|
|
||
| $this->assertSame('', (string) $this->pipeStream); | ||
| } | ||
|
|
||
| /** | ||
| * @covers Slim\Http\Stream::getContents | ||
| */ | ||
|
|
||
| public function testPipeGetContents() | ||
| { | ||
| $this->openPipeStream(); | ||
|
|
||
| $contents = trim($this->pipeStream->getContents()); | ||
| $this->assertSame('12', $contents); | ||
| } | ||
|
|
||
| /** | ||
| * Opens the pipe stream | ||
| * | ||
| * @see StreamTest::pipeStream | ||
| */ | ||
| private function openPipeStream() | ||
| { | ||
| $this->pipeFh = popen('echo 12', 'r'); | ||
| $this->pipeStream = new Stream($this->pipeFh); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add info on what this particular mode means?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expanded the comment