Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use HTTP/2 only for HTTPS requests #497

Merged
merged 1 commit into from
Jul 16, 2018
Merged

Conversation

franzliedke
Copy link
Contributor

The previously used flag tried to upgrade any request to HTTP/2, by sending an Upgrade: h2c header. Some HTTP servers (such as PHP's built-in one, which I use for mocking HTTP requests) apparently cannot deal with it. Using the CURL_HTTP_VERSION_2TLS flag instead makes Curl attempt an upgrade for HTTPS requests (e.g. to Stripe's API endpoint), but not for HTTP ones.

See https://ec.haxx.se/http-http2.html and https://curl.haxx.se/libcurl/c/CURLOPT_HTTP_VERSION.html.

(Plus, see my previous comments in PR #494 which introduced this feature.)

The previously used flag tried to upgrade *any* request to HTTP/2, by sending an `Upgrade: h2c` header. Some HTTP servers (such as PHP's built-in one, which I use for mocking HTTP requests) apparently cannot deal with it. Using the `CURL_HTTP_VERSION_2TLS` flag instead makes Curl attempt an upgrade for HTTPS requests (e.g. to Stripe's API endpoint), but not for HTTP ones.

See https://ec.haxx.se/http-http2.html and https://curl.haxx.se/libcurl/c/CURLOPT_HTTP_VERSION.html.
@franzliedke
Copy link
Contributor Author

Obligatory XKCD reference:

XKCD: This update broke my workflow

(from https://xkcd.com/1172/)

@ob-stripe
Copy link
Contributor

Aw, that's really unfortunate that PHP's builtin server does not handle this well :/

Thanks for looking into this and submitting a PR!

@ob-stripe ob-stripe merged commit 3f3a98b into stripe:master Jul 16, 2018
@ob-stripe
Copy link
Contributor

Released as 6.10.3.

@franzliedke
Copy link
Contributor Author

Thanks for the swift response. Works like a charm. 👍

@franzliedke franzliedke deleted the patch-1 branch July 16, 2018 11:58
@franzliedke
Copy link
Contributor Author

Another option, by the way, would be to make the Curl flag overridable. Right now, any of the user-providable default options will just be ignored - at least for this flag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants