search

php-http / message

HTTP Message related tools
http://php-http.org
MIT License
1.3k stars 42 forks source link

Add check for empty string to stream factories #70

Closed sagikazarmark closed 7 years ago

sagikazarmark commented 7 years ago
Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Related tickets fixes #69
Documentation does not apply
License MIT

What's in this PR?

Stream factories now check empty strings as well, not just null values.

dbu commented 7 years ago

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.

yes, but there is no type check to avoid objects. what exactly do we fix with this? is writing '' to a stream a bug? or is it just to be strict?

sagikazarmark commented 7 years ago

Well, I guess you can technically pass objects there. There is a string casting before writting.

is writing '' to a stream a bug? or is it just to be strict?

I don't think it's a bug. But we spare an unnecessary write (and now a rewind) operation. Not sure if adding an extra cast just to check if the string value of something is empty worth it.

dbu commented 7 years ago

if objects are not supposed to be passed, i think we are good here. if you incorrectly pass an object, the behaviour does not change compared to previous, and should still not fail.