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
Full request-line sent with http2 #3570
Comments
Is this allowed or supported in http2? libcurl reports a PROTOCOL_ERROR and nginx 1.10.3 server /var/log/nginx/error.log shows nothing. Lines 2513 to 2530 in f3294d9
If I change it to conn->httpversion != 20 then I can get it working [1] so it's possible. It looks like custom host headers would have to be ignored (or sent as actual :host?) to ensure that the actual host is always sent as :authority? I don't know the best way to address this so I tried it parsing the url in http2.c rather than try to predict or have logic whether or not we're at http2 elsewhere...feels wrong though diff --git a/lib/http2.c b/lib/http2.c
index 2884c71..5dd943e 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -1835,7 +1835,7 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex,
nghttp2_nv *nva = NULL;
size_t nheader;
size_t i;
- size_t authority_idx;
+ size_t authority_idx, path_idx, scheme_idx;
char *hdbuf = (char *)mem;
char *end, *line_end;
nghttp2_data_provider data_prd;
@@ -1947,6 +1947,7 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex,
}
if(!end || end == hdbuf)
goto fail;
+ path_idx = 1;
nva[1].name = (unsigned char *)":path";
nva[1].namelen = strlen((char *)nva[1].name);
nva[1].value = (unsigned char *)hdbuf;
@@ -1957,6 +1958,7 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex,
goto fail;
}
+ scheme_idx = 2;
nva[2].name = (unsigned char *)":scheme";
nva[2].namelen = strlen((char *)nva[2].name);
if(conn->handler->flags & PROTOPT_SSL)
@@ -2030,6 +2032,50 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex,
++i;
}
+ /* hack to fix http proxy over http2 */
+ if(conn->bits.httpproxy && !conn->bits.tunnel_proxy) {
+ if(!authority_idx || !path_idx || !scheme_idx) {
+ failf(conn->data, "http2 missing pseudo-headers for proxy modification");
+ goto fail;
+ }
+ for(i = 0; i < nva[path_idx].valuelen; ++i) {
+ if(nva[path_idx].value[i] == ':') {
+ if(i + 2 < nva[path_idx].valuelen &&
+ nva[path_idx].value[i+1] == '/' &&
+ nva[path_idx].value[i+2] == '/')
+ break;
+ else {
+ failf(conn->data, "http2 bad scheme in path for proxy modification");
+ goto fail;
+ }
+ }
+ }
+ if(i == 0 || i == nva[path_idx].valuelen) {
+ failf(conn->data, "http2 missing scheme in path for proxy modification");
+ goto fail;
+ }
+ nva[scheme_idx].valuelen = &nva[path_idx].value[i] -
+ nva[path_idx].value;
+ nva[scheme_idx].value = nva[path_idx].value;
+ nva[path_idx].valuelen -= &nva[path_idx].value[i+3] -
+ nva[path_idx].value;
+ nva[path_idx].value = &nva[path_idx].value[i+3];
+ for(i = 0; i < nva[path_idx].valuelen; ++i) {
+ if(nva[path_idx].value[i] == '/')
+ break;
+ }
+ if(i == 0 || i == nva[path_idx].valuelen) {
+ failf(conn->data, "http2 missing host in path for proxy modification");
+ goto fail;
+ }
+ nva[authority_idx].valuelen = &nva[path_idx].value[i] -
+ nva[path_idx].value;
+ nva[authority_idx].value = nva[path_idx].value;
+ nva[path_idx].valuelen -= &nva[path_idx].value[i] -
+ nva[path_idx].value;
+ nva[path_idx].value = &nva[path_idx].value[i];
+ }
+
/* :authority must come before non-pseudo header fields */
if(authority_idx != 0 && authority_idx != AUTHORITY_DST_IDX) {
nghttp2_nv authority = nva[authority_idx]; (Patch now up at branch jay:http2_proxy) [1]: I used a crude setup of nginx as a forward proxy with appended http2 to the listen line, added a |
I think the primary explanation for this flaw is the outstanding item in the TODO: "Support HTTP/2 for HTTP(S) proxies" This simply hasn't been made to work (yet)! |
- Parse the scheme, host and path from the :path component and then use that information to update the respective pseudo-headers. http2.c constructs an http2 request by parsing a regular http request that has been constructed in http.c. Normally this request has the path as the request-uri field (ie /foo in GET /foo HTTP/1.1) so that is what is used for http2 pseudo-header :path. However in the case of an http proxy libcurl uses the full URL as the request-uri field (ie http://test.com/foo in GET http://test.com/foo HTTP/1.1) so that is not just :path but must be split into :scheme and :authority (host) which is what this patch does. A side effect of this is that any user-specified host field is overridden. Ref: curl#3570 (comment) Ref: https://curl.haxx.se/docs/todo.html#Support_HTTP_2_for_HTTP_S_proxi
I also ran into this today. Noted at nghttp2/nghttp2#1304 (comment) I'm not entirely sure what the behaviour here is meant to be, so in lua-http I added: https://github.com/daurnimator/lua-http/pull/141/files#diff-b4a19dcf32626be03ed2854d0f44ff8eR457 |
I close this issue now and refer to the TODO issue again for our still outstanding task to add HTTP/2 support for proxies. The @jay's comments and work above can certainly serve as the beginning of this support, but should be submitted as a regular PR instead when ripe and we can then discuss its merits and the details then. |
- Do not switch to HTTP/2 for an HTTP proxy that is not tunnelling to the destination host. We already do something similar for HTTPS proxies by not sending h2. [1] Prior to this change setting CURL_HTTP_VERSION_2_PRIOR_KNOWLEDGE would incorrectly use HTTP/2 to talk to the proxy, which is not something we support (yet?). Also it's debatable whether or not that setting should apply to HTTP/2 proxies. [1]: curl@17c5d05 Bug: curl#3570 Bug: curl#3832 Closes #xxxx
- Do not switch to HTTP/2 for an HTTP proxy that is not tunnelling to the destination host. We already do something similar for HTTPS proxies by not sending h2. [1] Prior to this change setting CURL_HTTP_VERSION_2_PRIOR_KNOWLEDGE would incorrectly use HTTP/2 to talk to the proxy, which is not something we support (yet?). Also it's debatable whether or not that setting should apply to HTTP/2 proxies. [1]: 17c5d05 Bug: #3570 Bug: #3832 Closes #3853
I did this
and from that I got this (the relevant part only):
Which my Nginx server terminated with:
I expected the following
With
http2
the client should not send other than path+query with the request line as it is not supported withhttp2
.I expected:
curl/libcurl version
operating system
macOS Sierra 10.12.6
The text was updated successfully, but these errors were encountered: