Enable HTTP/2 support

[ob-stripe]

r? @brandur-stripe
cc @stripe/api-libraries

The API doesn't support HTTP/2 yet, but it will Soon™.

This should be safe to merge now. Per curl's documentation:

libcurl will fall back to HTTP 1.1 if HTTP 2 can't be negotiated with the server.

[brandur-stripe]

Whoah, cool!

LGTM, but it looks like a couple of the builds in the matrix failed with errors around connectivity problems — it may or may not be an indicator that cURL didn't fall back as suggested, and worth looking into.

ptal @ob-stripe

[ob-stripe]

I don't think this is related. We've been observing these issues sporadically for the past few days. One of the tests use httpbin.org, which seems to be having some trouble lately (it's returning a 503 for me right now).

[ob-stripe]

That said, we should probably fix or get rid of this test if it's making the test suite brittle.

[brandur-stripe]

I don't think this is related. We've been observing these issues sporadically for the past few days. One of the tests use httpbin.org, which seems to be having some trouble lately (it's returning a 503 for me right now).

Ah, I see! That would make more sense.

We might be able to get stripe-mock serving over HTTP2 too (probably as an option) as well since the Go support is pretty good. The only complicated part is that it mandates HTTPS/certificates which adds a little complication.

[brandur-stripe]

I just restarted the matrix entries that failed.

LGTM.

[franzliedke]

Interestingly, I am encountering similar problems in my tests as well - starting with the upgrade to v6.10.2. The error message I see is (Network error [errno 52]: Empty reply from server).

I am using this library to mock my HTTP interaction with Stripe.

[franzliedke]

If I understand this page correctly, using CURL_HTTP_VERSION_2_0 with HTTP (i.e. not HTTPS) connections can cause problems (which might be what I am experiencing here). And indeed, using CURL_HTTP_VERSION_2TLS works for me. Probably because it will not try to use HTTP/2 with my HTTP connection to the mock server. It will still use it with Stripe's official API endpoint, which is served via HTTPS.

[GrahamCampbell]

You claim to be using PSR-4 autoloading in the composer.json, but defining constants like this is a strong violation of the spec.

[franzliedke]

@GrahamCampbell I see no reference to constants in either PSR-0 or PSR-4. Can you clarify what you mean?

That said, not sure porting the constant back for releases of ext-curl that don't have it... doesn't that mean that the underlying Curl library won't have it, either? Maybe something like this would be less "dangerous":

if (defined(CURL_HTTP_VERSION_2_0)) {
    $opts[CURLOPT_HTTP_VERSION] = CURL_HTTP_VERSION_2_0;
}

[ob-stripe]

The issue is that it is possible to use an older PHP version (e.g. 5.4) where the curl constants are not defined with a newer ext-curl version that does support features like HTTP/2.

Thankfully, curl is robust enough that providing an unknown value for a configuration option simply results in curl using the default value for that option, so it is safe to pass these values regardless of which ext-curl version is used.

Re. PSR-4, I'm also not sure that what we do in the library is in violation of the spec. I assume it's because the constants are not constrained to the \Stripe namespace? We could "fix" that, but I'm not sure it's worth the trouble. In practice, what are the chances that someone would define constants with the same names but different values in their own code?