php-http / message

HTTP Message related tools

http://php-http.org

MIT License

1.29k stars 41 forks source link

Fix encoding stream size #61

Closed ajgarlag closed 7 years ago

ajgarlag commented 7 years ago

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Related tickets	fixes #60
License	MIT

What's in this PR?

This PR includes the tests required to expose the problem described in #60 about the invalid getSize returned value by FilteredStream subclasses, and should include a fix.

Why?

60 Invalid value returned by FilteredStream::getSize

Checklist

[x] Updated CHANGELOG.md to describe BC breaks / deprecations | new feature | bugfix
To Do
[x] Discuss and find a solution to fix the tests
[x] Squash commits

ajgarlag commented 7 years ago

A quick fix could be to implement a getSize method in FilteredStream to always return null (allowed by PSR-7 when the size is unknown) and rewrite the tests to expect that null value.

joelwurtz commented 7 years ago

AFAIK, result size on the compression cannot be determined, but they may be headers for the decompression (not sure).

So :+1: for the null value as i don't see a way to do this unless we read up the whole stream ^^ which will make it non readable as we cannot seek a filtered stream (don't remember where is see this but using \php_user_filter make a stream not seekable)

dbu commented 7 years ago

that looks at least not wrong to me. i am :+1: to merge this. for future reference, can we do better than this for some of the stream types?

joelwurtz commented 7 years ago

LGTM,

Could you rebase with the last PR merged (to remove conflict in the changelog), then it's fine for me

sagikazarmark commented 7 years ago

Sorry for the late response. Thanks.