PHP 5.6: Remove raw body callback, php://input is seekable now

[Majkl578]

BC break (deprecation of $rawBodyCallback in constructor)
I'd also remove NULL on empty data and return empty string, but I'm not sure about implications in user-land...

Depends on #70 (composer.json & Travis).

[JanTvrdik]

👎 The previous implementation was cleaner. Http\Request should not directly read php://input.

[Majkl578]

Cleaner in what way? It just always directly called php://input, also it was not type safe (no signature contract).

[JanTvrdik]

Request should not read global state.

[jkuchar]

Agree, current implementation is easier to mock.

[Majkl578]

I agree that it should not read global state.

But current implementation is just a hack because php://input was not rewindable in <5.6. Closure callback is not type safe, could not be properly type hinted and relied upon (especially in PHP 7) or unit tested on its own. (Unit tests for closure? Sounds weird.)

I also had another solution in mind, except obvious option to introduce an interface (sounds unnecessary), an option to provide a stream address php://input via constructor. It could be easily mocked/tested as well. And also provide an option to return it as a resource instead of string copy (this is also what Symfony supports).
But I think it is out of scope of this PR.

[JanTvrdik]

But current implementation is just a hack because

Nope. I wrote the current implementation and the callback is here because "Request should not read global state". The hack was there before the callback.

Closure callback is not type safe

Which noone really cares about, this is PHP. Request is low-level class, most people don't care about the constructor.

[Majkl578]

If you don't care about constructor, why not just pass resource then?

[JanTvrdik]

That was considered, but doing so in PHP < 5.6 is AFAIK very difficult to do reliably and efficiently. Doing so now is an unnecessary BC break for no clear gain.

[Majkl578]

Sure, it would be a little BC break (funny, considering all the PHP 5.6/7.0 changes that are coming), but as you said, for a low level class where almost noone cares about the constructor.

[JanTvrdik]

But what would we gain with such BC break? Support for processing large post bodies? Maybe.

Assuming there is no performance penalty for passing STDIN constant, we may add getRawBodyStream() which would return resource and keep getRawBody() which will return string with stream_get_contents. Note that you can not typehint resource either.

[dg]

Implemented in this way 0f5cee3