opened
dshafik
username_0
As per [the RFC](https://wiki.php.net/rfc/curl_http2_push)
Add HTTP/2 Server Push to ext/curl
issue
created
jpauli
username_1
Hi.

Could we add some tests in this PR please ?
comment
created
dshafik
username_0
@username_1 I can double check/add tests for the refactoring a, but the curl push stuff requires an HTTP/2 capable server with push support; do you have any thoughts on how to test it? 

Currently I have a docker container I spin up with both a node.js and nghttpd test servers on it, but that obviously isn't something we'd want to port over into the php test suite.
comment
created
jpauli
username_1
I have no idea, but I'm sceptic at merging a PR that adds some code like this one, with no test at all.
comment
created
dshafik
username_0
@username_1 I agree, but it will need discussion to figure out the best way to handle it. We should probably figure out a way for all HTTP/2 related functionality – in the future that might be the http stream and other extensions.
comment
created
jpauli
username_1
Does the RFC point that ? If not, perhaps it is the moment to open the discussion ?
Tests also have the advantage to reveal eventual memory leaks or bad memory accesses (using valgrind mainly)
comment
created
rlerdorf
username_2
We could think about adding HTTP/2 to the built-in server to have a native test target.
comment
created
dshafik
username_0
@username_2 that's not a bad idea but also a big undertaking - I'm not sure we could even do push as it is blocking AFAIK
comment
created
php-pulls
username_3
I don't see that as being a limitation. You can obviously write a single-threaded event-driven HTTP/2 implementation. In fact that is the best way to implement it. But yes, it is non-trivial. Unless we can find a nice library that does all the heavy lifting that we can just link against.
comment
created
dshafik
username_0
We could probably use nghttp2 for all the heavy lifting, it's a pretty fantastic library - but it's another dependency for core, of course. Perhaps make it conditional so that it will serve HTTP/1 regardless?
comment