On Sat, Feb 1, 2014 at 1:15 AM, Tatsuhiro Tsujikawa <[email protected]>wrote:
> > > > On Fri, Jan 31, 2014 at 8:14 PM, Fabian Frank > <[email protected]>wrote: > >> >> On Jan 30, 2014, at 2:50 PM, Daniel Stenberg <[email protected]> wrote: >> >> > 1 - send the request using plain HTTP2 HEADERS when connecting over >> https:// >> > (and when using re-used http2) >> I have worked on this today and the current progress is attached. Unless >> someone wants to build on top of it, no need to merge, I plan to continue >> cleaning it up and handle stopping the HTTP1 logic correctly as well as >> swapping the send/receive callbacks correctly for SSL connections. >> >> > Good progress. client header and SETTINGS submissions look good. > > Coincidentally, I did same approach. Patch is attached below. > The idea is mostly the same, I add some extra code such as converting HTTP > header from curl to HTTP2 format and calls underlying recv/send callback > including TLS and non-TLS. > > I send client header and request in http2_send, but this is because I have > no idea where http_conn is initialized. As I commented in the code, it is > cleaner to add dedicated function for HTTP2 request, since http2_send is > also used upload. > > Anyway, with this patch, one can transfer contents from > https://twitter.com > and from nghttp2 test server in plain HTTP as well, which is pretty good. > > The code still has rough edges. The notable one is I could not > figure out how to call nghttp2_session_send() when underlying > socket is writable. > > I took the liberty of going forward and made additional improvements. The patch attached. 0002: This addresses the (5) in http://curl.haxx.se/mail/lib-2014-02/0001.html I reused Curl_send_buffer struct to save the line of code, but the idea is we just need buffers to store incoming header blocks and search for :status header to correctly construct status line in HTTP/1 format. 0003: This is basically nghttp2 specific patch. Currently, we only care about a single stream. But the callbacks are called for the rest of the stream. This patch ensures that we process incoming frame for stream we care about. 0004: We moved inbuf from http2_recv to http_conn struct so that we can handle the data in on_data_chunk_recv than remaining buffer. Feel free to pick one or some hunks from my patch. I'm also willing to rebase. Best regards, Tatsuhiro Tsujikawa
From 0a0edbe17b7214e820d59fffe387d7703da97b64 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa <[email protected]> Date: Sun, 2 Feb 2014 21:03:18 +0900 Subject: [PATCH 2/4] Store response header in temporary buffer --- lib/http.h | 3 +++ lib/http2.c | 63 ++++++++++++++++++++++++++++++++++++++++--------------------- 2 files changed, 45 insertions(+), 21 deletions(-) diff --git a/lib/http.h b/lib/http.h index d0a4138..5907ff3 100644 --- a/lib/http.h +++ b/lib/http.h @@ -161,6 +161,9 @@ struct http_conn { void *send_underlying; /* underlying send Curl_send callback */ void *recv_underlying; /* underlying recv Curl_recv callback */ bool closed; /* TRUE on HTTP2 stream close */ + Curl_send_buffer *header_recvbuf; /* store response headers */ + size_t nread_header_recvbuf; /* number of bytes in header_recvbuf + fed into upper layer */ #else int unused; /* prevent a compiler warning */ #endif diff --git a/lib/http2.c b/lib/http2.c index 0b70ef5..34ac915 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -116,10 +116,23 @@ static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame, void *userp) { struct connectdata *conn = (struct connectdata *)userp; + struct http_conn *c = &conn->proto.httpc; (void)session; (void)frame; infof(conn->data, "on_frame_recv() was called with header %x\n", frame->hd.type); + if(frame->hd.type == NGHTTP2_HEADERS && + frame->headers.cat == NGHTTP2_HCAT_RESPONSE) { + c->bodystarted = TRUE; + Curl_add_buffer(c->header_recvbuf, "\r\n", 2); + c->nread_header_recvbuf = c->len < c->header_recvbuf->size_used ? + c->len : c->header_recvbuf->size_used; + + memcpy(c->mem, c->header_recvbuf->buffer, c->nread_header_recvbuf); + + c->mem += c->nread_header_recvbuf; + c->len -= c->nread_header_recvbuf; + } if((frame->hd.type == NGHTTP2_HEADERS || frame->hd.type == NGHTTP2_DATA) && frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { infof(conn->data, "stream_id=%d closed\n", frame->hd.stream_id); @@ -151,13 +164,6 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags, infof(conn->data, "on_data_chunk_recv() " "len = %u, stream = %x\n", len, stream_id); - if(!c->bodystarted) { - memcpy(c->mem, "\r\n", 2); /* signal end of headers */ - c->mem += 2; - c->len -= 2; - c->bodystarted = TRUE; - } - if(len <= c->len) { memcpy(c->mem, data, len); c->mem += len; @@ -241,6 +247,8 @@ static int on_begin_headers(nghttp2_session *session, return 0; } +static const char STATUS[] = ":status"; + /* frame->hd.type is either NGHTTP2_HEADERS or NGHTTP2_PUSH_PROMISE */ static int on_header(nghttp2_session *session, const nghttp2_frame *frame, const uint8_t *name, size_t namelen, @@ -249,26 +257,23 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame, { struct connectdata *conn = (struct connectdata *)userp; struct http_conn *c = &conn->proto.httpc; - size_t hlen = namelen + valuelen + 3; /* colon + CRLF == 3 bytes */ - (void)session; (void)frame; - if(namelen && (name[0] == ':')) { - /* special case */ - hlen = snprintf(c->mem, c->len, "HTTP/2.0 %s\r\n", value); + if(namelen == sizeof(":status") - 1 && + memcmp(STATUS, name, namelen) == 0) { + snprintf(c->header_recvbuf->buffer, 13, "HTTP/2.0 %s", value); + c->header_recvbuf->buffer[12] = '\r'; + return 0; } - else if(hlen + 1 < c->len) { /* hlen + a zero byte */ + else { /* convert to a HTTP1-style header */ - memcpy(c->mem, name, namelen); - c->mem[namelen]=':'; - memcpy(&c->mem[namelen+1], value, valuelen); - c->mem[namelen + valuelen + 1]='\r'; - c->mem[namelen + valuelen + 2]='\n'; - c->mem[namelen + valuelen + 3]=0; /* to display this easier */ + infof(conn->data, "got header\n"); + Curl_add_buffer(c->header_recvbuf, name, namelen); + Curl_add_buffer(c->header_recvbuf, ":", 1); + Curl_add_buffer(c->header_recvbuf, value, valuelen); + Curl_add_buffer(c->header_recvbuf, "\r\n", 2); } - c->mem += hlen; - c->len -= hlen; return 0; /* 0 is successful */ } @@ -388,6 +393,17 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex, (void)sockindex; /* we always do HTTP2 on sockindex 0 */ + if(httpc->bodystarted && + httpc->nread_header_recvbuf < httpc->header_recvbuf->size_used) { + size_t left = + httpc->header_recvbuf->size_used - httpc->nread_header_recvbuf; + size_t nread = len < left ? len : left; + memcpy(mem, httpc->header_recvbuf->buffer + httpc->nread_header_recvbuf, + nread); + httpc->nread_header_recvbuf += nread; + return nread; + } + conn->proto.httpc.mem = mem; conn->proto.httpc.len = len; @@ -567,6 +583,11 @@ int Curl_http2_switched(struct connectdata *conn) infof(conn->data, "We have switched to HTTP2\n"); httpc->bodystarted = FALSE; httpc->closed = FALSE; + httpc->header_recvbuf = Curl_add_buffer_init(); + httpc->nread_header_recvbuf = 0; + + /* Put place holder for status line */ + Curl_add_buffer(httpc->header_recvbuf, "HTTP/2.0 200\r\n", 14); /* TODO: May get CURLE_AGAIN */ rv = ((Curl_send*)httpc->send_underlying) -- 1.8.4.2
From 50e15de56a7d25c5f97d5676a541c39ed1609734 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa <[email protected]> Date: Sun, 2 Feb 2014 21:31:13 +0900 Subject: [PATCH 3/4] Check stream ID we are interested in --- lib/http.h | 1 + lib/http2.c | 41 +++++++++++++++++++++++++++++++++++------ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/lib/http.h b/lib/http.h index 5907ff3..404cca8 100644 --- a/lib/http.h +++ b/lib/http.h @@ -164,6 +164,7 @@ struct http_conn { Curl_send_buffer *header_recvbuf; /* store response headers */ size_t nread_header_recvbuf; /* number of bytes in header_recvbuf fed into upper layer */ + int32_t stream_id; /* stream we are interested in */ #else int unused; /* prevent a compiler warning */ #endif diff --git a/lib/http2.c b/lib/http2.c index 34ac915..fe5d5ac 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -117,12 +117,15 @@ static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame, { struct connectdata *conn = (struct connectdata *)userp; struct http_conn *c = &conn->proto.httpc; + int rv; (void)session; (void)frame; infof(conn->data, "on_frame_recv() was called with header %x\n", frame->hd.type); - if(frame->hd.type == NGHTTP2_HEADERS && - frame->headers.cat == NGHTTP2_HCAT_RESPONSE) { + switch(frame->hd.type) { + case NGHTTP2_HEADERS: + if(frame->headers.cat != NGHTTP2_HCAT_RESPONSE) + break; c->bodystarted = TRUE; Curl_add_buffer(c->header_recvbuf, "\r\n", 2); c->nread_header_recvbuf = c->len < c->header_recvbuf->size_used ? @@ -132,10 +135,14 @@ static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame, c->mem += c->nread_header_recvbuf; c->len -= c->nread_header_recvbuf; - } - if((frame->hd.type == NGHTTP2_HEADERS || frame->hd.type == NGHTTP2_DATA) && - frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { - infof(conn->data, "stream_id=%d closed\n", frame->hd.stream_id); + break; + case NGHTTP2_PUSH_PROMISE: + rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, + frame->hd.stream_id, NGHTTP2_CANCEL); + if(nghttp2_is_fatal(rv)) { + return rv; + } + break; } return 0; } @@ -164,6 +171,10 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags, infof(conn->data, "on_data_chunk_recv() " "len = %u, stream = %x\n", len, stream_id); + if(stream_id != c->stream_id) { + return 0; + } + if(len <= c->len) { memcpy(c->mem, data, len); c->mem += len; @@ -182,9 +193,15 @@ static int before_frame_send(nghttp2_session *session, void *userp) { struct connectdata *conn = (struct connectdata *)userp; + struct http_conn *c = &conn->proto.httpc; (void)session; (void)frame; infof(conn->data, "before_frame_send() was called\n"); + if(frame->hd.type == NGHTTP2_HEADERS && + frame->headers.cat == NGHTTP2_HCAT_REQUEST) { + /* Get stream ID of our request */ + c->stream_id = frame->hd.stream_id; + } return 0; } static int on_frame_send(nghttp2_session *session, @@ -218,6 +235,10 @@ static int on_stream_close(nghttp2_session *session, int32_t stream_id, infof(conn->data, "on_stream_close() was called, error_code = %d\n", error_code); + if(stream_id != c->stream_id) { + return 0; + } + c->closed = TRUE; return 0; @@ -260,6 +281,10 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame, (void)session; (void)frame; + if(frame->hd.stream_id != c->stream_id) { + return 0; + } + if(namelen == sizeof(":status") - 1 && memcmp(STATUS, name, namelen) == 0) { snprintf(c->header_recvbuf->buffer, 13, "HTTP/2.0 %s", value); @@ -597,6 +622,8 @@ int Curl_http2_switched(struct connectdata *conn) &rc); assert(rv == 24); if(conn->data->req.upgr101 == UPGR101_RECEIVED) { + /* stream 1 is opened implicitly on upgrade */ + httpc->stream_id = 1; /* queue SETTINGS frame (again) */ rv = nghttp2_session_upgrade(httpc->h2, httpc->binsettings, httpc->binlen, NULL); @@ -606,6 +633,8 @@ int Curl_http2_switched(struct connectdata *conn) return -1; } } else { + /* stream ID is unknown at this point */ + httpc->stream_id = -1; rv = nghttp2_submit_settings(httpc->h2, NGHTTP2_FLAG_NONE, NULL, 0); if(rv != 0) { failf(conn->data, "nghttp2_submit_settings() failed: %s(%d)", -- 1.8.4.2
From ec51a4a8b9900e698a3a093333ecfff6e930dafb Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa <[email protected]> Date: Sun, 2 Feb 2014 23:01:54 +0900 Subject: [PATCH 4/4] Deal with data larger than remaining buffer --- lib/http.h | 4 ++++ lib/http2.c | 53 +++++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/lib/http.h b/lib/http.h index 404cca8..8b7535f 100644 --- a/lib/http.h +++ b/lib/http.h @@ -165,6 +165,10 @@ struct http_conn { size_t nread_header_recvbuf; /* number of bytes in header_recvbuf fed into upper layer */ int32_t stream_id; /* stream we are interested in */ + const uint8_t *data; /* pointer to data chunk, received in + on_data_chunk */ + size_t datalen; /* the number of bytes left in data */ + char *inbuf; /* buffer to receive data from underlying socket */ #else int unused; /* prevent a compiler warning */ #endif diff --git a/lib/http2.c b/lib/http2.c index fe5d5ac..4fec5a5 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -165,6 +165,7 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags, { struct connectdata *conn = (struct connectdata *)userp; struct http_conn *c = &conn->proto.httpc; + size_t nread; (void)session; (void)flags; (void)data; @@ -175,16 +176,19 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags, return 0; } - if(len <= c->len) { - memcpy(c->mem, data, len); - c->mem += len; - c->len -= len; - } - else { - infof(conn->data, "EEEEEEK: %d > %d\n", len, c->len); - /* return NGHTTP2_ERR_PAUSE; */ - } + nread = c->len < len ? c->len : len; + memcpy(c->mem, data, nread); + + c->mem += nread; + c->len -= nread; + + infof(conn->data, "%zu data written\n", nread); + if(nread < len) { + c->data = data + nread; + c->datalen = len - nread; + return NGHTTP2_ERR_PAUSE; + } return 0; } @@ -329,11 +333,17 @@ static nghttp2_settings_entry settings[] = { { NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE, NGHTTP2_INITIAL_WINDOW_SIZE }, }; +#define H2_BUFSIZE 4096 + /* * Initialize nghttp2 for a Curl connection */ CURLcode Curl_http2_init(struct connectdata *conn) { if(!conn->proto.httpc.h2) { + conn->proto.httpc.inbuf = malloc(H2_BUFSIZE); + if(conn->proto.httpc.inbuf == NULL) { + return CURLE_OUT_OF_MEMORY; + } /* The nghttp2 session is not yet setup, do it */ int rc = nghttp2_session_client_new(&conn->proto.httpc.h2, &callbacks, conn); @@ -401,8 +411,6 @@ CURLcode Curl_http2_request_upgrade(Curl_send_buffer *req, return result; } -#define H2_BUFSIZE 4096 - /* * If the read would block (EWOULDBLOCK) we return -1. Otherwise we return * a regular CURLcode value. @@ -413,7 +421,6 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex, CURLcode rc; ssize_t rv; ssize_t nread; - char inbuf[H2_BUFSIZE]; struct http_conn *httpc = &conn->proto.httpc; (void)sockindex; /* we always do HTTP2 on sockindex 0 */ @@ -429,6 +436,21 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex, return nread; } + if(httpc->data) { + nread = len < httpc->datalen ? len : httpc->datalen; + memcpy(mem, httpc->data, nread); + + httpc->data += nread; + httpc->datalen -= nread; + + infof(conn->data, "%zu data written\n", nread); + if(httpc->datalen == 0) { + httpc->data = NULL; + httpc->datalen = 0; + } + return nread; + } + conn->proto.httpc.mem = mem; conn->proto.httpc.len = len; @@ -437,7 +459,7 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex, rc = 0; nread = ((Curl_recv*)httpc->recv_underlying)(conn, FIRSTSOCKET, - inbuf, H2_BUFSIZE, &rc); + httpc->inbuf, H2_BUFSIZE, &rc); if(rc == CURLE_AGAIN) { *err = rc; @@ -451,7 +473,8 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex, } infof(conn->data, "nread=%zd\n", nread); - rv = nghttp2_session_mem_recv(httpc->h2, (const uint8_t *)inbuf, nread); + rv = nghttp2_session_mem_recv(httpc->h2, + (const uint8_t *)httpc->inbuf, nread); if(nghttp2_is_fatal((int)rv)) { failf(conn->data, "nghttp2_session_mem_recv() returned %d:%s\n", @@ -610,6 +633,8 @@ int Curl_http2_switched(struct connectdata *conn) httpc->closed = FALSE; httpc->header_recvbuf = Curl_add_buffer_init(); httpc->nread_header_recvbuf = 0; + httpc->data = NULL; + httpc->datalen = 0; /* Put place holder for status line */ Curl_add_buffer(httpc->header_recvbuf, "HTTP/2.0 200\r\n", 14); -- 1.8.4.2
------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.html
