Skip to content

Add check for empty string to stream factories #70

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jan 8, 2017
Merged

Add check for empty string to stream factories #70

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5265147
Add check for empty string to stream factories
sagikazarmark Jan 4, 2017
e9c1041
Return stream when body is empty
sagikazarmark Jan 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@

## Unreleased

## Added

- Check for empty string in Stream factories

## Fixed

- FilteredStream::getSize returns null because the contents size is unknown.
Expand Down
2 changes: 1 addition & 1 deletion src/StreamFactory/DiactorosStreamFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public function createStream($body = null)
} else {
$stream = new Stream('php://memory', 'rw');

if (null !== $body) {
if (null !== $body || '' !== $body) {
Copy link
Member

Choose a reason for hiding this comment

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

Why logical or? We should write to the stream if the body has content.

if (null !== $body && '' !== $body) {

Copy link
Contributor

Choose a reason for hiding this comment

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

we could also do if (null === $body || '' === $body) {return new Stream('php://memory', 'rw');} as the rest of the code is irrelevant if the body is empty

Copy link
Member

Choose a reason for hiding this comment

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

Also this doesn't work if body is an object and casting the object to string return an empty value. We should do the following :

$content = (string) $body;

if (strlen($content) > 0) {
    $stream->write($content);
}

return $stream

Copy link
Member Author

Choose a reason for hiding this comment

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

@dbu indeed, an empty stream does not have to be rewinded, or does it?

@joelwurtz According to the interface the allowed types are string|resource|StreamInterface|null, so an object would not be valid in the first place, you would have to cast it to string first.

$stream->write((string) $body);
}

Expand Down
2 changes: 1 addition & 1 deletion src/StreamFactory/SlimStreamFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public function createStream($body = null)
$resource = fopen('php://memory', 'r+');
$stream = new Stream($resource);

if (null !== $body) {
if (null !== $body || '' !== $body) {
Copy link
Member

Choose a reason for hiding this comment

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

Same here. Use &&

$stream->write((string) $body);
}
}
Expand Down