From 67d5b6821b13c8fb3bc7c0ed3a25b98d4f103de7 Mon Sep 17 00:00:00 2001 From: Lucas Pardue Date: Mon, 24 Apr 2017 18:09:16 +0100 Subject: [PATCH 1/7] ORIGIN frame support --- lib/includes/nghttp2/nghttp2.h | 67 +++++++++++++++++++++++++++++++- lib/nghttp2_frame.c | 20 ++++++++++ lib/nghttp2_frame.h | 21 +++++++++- lib/nghttp2_option.c | 4 ++ lib/nghttp2_session.c | 36 +++++++++++++++++ lib/nghttp2_session.h | 3 +- lib/nghttp2_submit.c | 70 ++++++++++++++++++++++++++++++++++ 7 files changed, 218 insertions(+), 3 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 15901004..172de81a 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -597,7 +597,12 @@ typedef enum { * The ALTSVC frame, which is defined in `RFC 7383 * `_. */ - NGHTTP2_ALTSVC = 0x0a + NGHTTP2_ALTSVC = 0x0a, + /** + * The ORIGIN frame, whic is defined in `RFC XXXX + * https://tools.ietf.org/html/draft-ietf-httpbis-origin-frame-03#section-2`_. + */ + NGHTTP2_ORIGIN = 0x0c } nghttp2_frame_type; /** @@ -4471,6 +4476,66 @@ NGHTTP2_EXTERN int nghttp2_submit_altsvc(nghttp2_session *session, const uint8_t *field_value, size_t field_value_len); +/** + * @struct + * + * The payload of ORIGIN frame. ORIGIN frame is a non-critical + * extension to HTTP/2. If this frame is received, and + * `nghttp2_option_set_user_recv_extension_type()` is not set, and + * `nghttp2_option_set_builtin_recv_extension_type()` is set for + * :enum:`NGHTTP2_ORIGIN`, ``nghttp2_extension.payload`` will point to + * this struct. + * + * It has the following members: + */ +typedef struct { + /** + * The pointer to an origin which the sender believes this connection is or + * could be + * authoritative for. This is not necessarily NULL-terminated. + */ + uint8_t *origin; + /** + * The length of the |origin|. + */ + size_t origin_len; +} nghttp2_ext_origin; + +/** + * @function + * + * Submits ORIGIN frame. + * + * ORIGIN frame is a non-critical extension to HTTP/2, and defined in + * `RFC XXX + * `_. + * + * The |flags| is currently ignored and should be + * :enum:`NGHTTP2_FLAG_NONE`. + * + * The |origin| points to the origin which the sender believes this connection + * is or could be + * authoritative for + * + * The ORIGIN frame is only usable from server side. If this function + * is invoked with client side session, this function returns + * :enum:`NGHTTP2_ERR_INVALID_STATE`. + * + * This function returns 0 if it succeeds, or one of the following + * negative error codes: + * + * :enum:`NGHTTP2_ERR_NOMEM` + * Out of memory + * :enum:`NGHTTP2_ERR_INVALID_STATE` + * The function is called from client side session + * :enum:`NGHTTP2_ERR_INVALID_ARGUMENT` + * The |origin_len| is larger than + * 16382. + */ +NGHTTP2_EXTERN int nghttp2_submit_origin(nghttp2_session *session, + uint8_t flags, const uint8_t *origin, + size_t origin_len); + /** * @function * diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c index 90efaff5..84bdb692 100644 --- a/lib/nghttp2_frame.c +++ b/lib/nghttp2_frame.c @@ -196,6 +196,26 @@ void nghttp2_frame_extension_init(nghttp2_extension *frame, uint8_t type, void nghttp2_frame_extension_free(nghttp2_extension *frame) { (void)frame; } +void nghttp2_frame_origin_init(nghttp2_extension *frame, uint8_t *origin, + size_t origin_len) { + nghttp2_ext_origin *origin_frame; + + /* Always send ORIGIN frame on stream 0 */ + nghttp2_frame_hd_init(&frame->hd, 2 + origin_len, NGHTTP2_ORIGIN, + NGHTTP2_FLAG_NONE, 0); + + origin_frame = frame->payload; + origin_frame->origin = origin; + origin_frame->origin_len = origin_len; +} + +void nghttp2_frame_origin_free(nghttp2_extension *frame, nghttp2_mem *mem) { + nghttp2_ext_origin *origin_frame; + + origin_frame = frame->payload; + nghttp2_mem_free(mem, origin_frame->origin); +} + void nghttp2_frame_altsvc_init(nghttp2_extension *frame, int32_t stream_id, uint8_t *origin, size_t origin_len, uint8_t *field_value, size_t field_value_len) { diff --git a/lib/nghttp2_frame.h b/lib/nghttp2_frame.h index 891289f6..d15d3890 100644 --- a/lib/nghttp2_frame.h +++ b/lib/nghttp2_frame.h @@ -70,7 +70,10 @@ #define NGHTTP2_MAX_PADLEN 256 /* Union of extension frame payload */ -typedef union { nghttp2_ext_altsvc altsvc; } nghttp2_ext_frame_payload; +typedef union { + nghttp2_ext_altsvc altsvc; + nghttp2_ext_origin origin; +} nghttp2_ext_frame_payload; void nghttp2_frame_pack_frame_hd(uint8_t *buf, const nghttp2_frame_hd *hd); @@ -487,6 +490,22 @@ void nghttp2_frame_altsvc_init(nghttp2_extension *frame, int32_t stream_id, */ void nghttp2_frame_altsvc_free(nghttp2_extension *frame, nghttp2_mem *mem); +/* + * Initializes ORIGIN frame |frame| with given values. This function + * assumes that frame->payload points to nghttp2_ext_origin object. + * On success, this function takes ownership of + * |origin|, so caller must not free it. + */ +void nghttp2_frame_origin_init(nghttp2_extension *frame, uint8_t *origin, + size_t origin_len); + +/* + * Frees up resources under |frame|. This function does not free + * nghttp2_ext_origin object pointed by frame->payload. This function + * only frees origin pointed by nghttp2_ext_origin.origin. + */ +void nghttp2_frame_origin_free(nghttp2_extension *frame, nghttp2_mem *mem); + /* * Returns the number of padding bytes after payload. The total * padding length is given in the |padlen|. The returned value does diff --git a/lib/nghttp2_option.c b/lib/nghttp2_option.c index aec5dcfa..8946d7dd 100644 --- a/lib/nghttp2_option.c +++ b/lib/nghttp2_option.c @@ -86,6 +86,10 @@ void nghttp2_option_set_builtin_recv_extension_type(nghttp2_option *option, option->opt_set_mask |= NGHTTP2_OPT_BUILTIN_RECV_EXT_TYPES; option->builtin_recv_ext_types |= NGHTTP2_TYPEMASK_ALTSVC; return; + case NGHTTP2_ORIGIN: + option->opt_set_mask |= NGHTTP2_OPT_BUILTIN_RECV_EXT_TYPES; + option->builtin_recv_ext_types |= NGHTTP2_TYPEMASK_ORIGIN; + return; default: return; } diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 1c060f1b..7810ae08 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -337,6 +337,12 @@ static void session_inbound_frame_reset(nghttp2_session *session) { } nghttp2_frame_altsvc_free(&iframe->frame.ext, mem); break; + case NGHTTP2_ORIGIN: + if ((session->builtin_recv_ext_types & NGHTTP2_TYPEMASK_ORIGIN) == 0) { + break; + } + nghttp2_frame_origin_free(&iframe->frame.ext, mem); + break; } } @@ -5726,7 +5732,37 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in, iframe->state = NGHTTP2_IB_READ_NBYTE; inbound_frame_set_mark(iframe, 2); + case NGHTTP2_ORIGIN: + if ((session->builtin_recv_ext_types & NGHTTP2_TYPEMASK_ORIGIN) == + 0) { + busy = 1; + iframe->state = NGHTTP2_IB_IGN_PAYLOAD; + break; + } + DEBUGF("recv: ORIGIN\n"); + + iframe->frame.hd.flags = NGHTTP2_FLAG_NONE; + iframe->frame.ext.payload = &iframe->ext_frame_payload.origin; + + if (session->server) { + busy = 1; + iframe->state = NGHTTP2_IB_IGN_PAYLOAD; + break; + } + + if (iframe->payloadleft < 2) { + busy = 1; + iframe->state = NGHTTP2_IB_FRAME_SIZE_ERROR; + break; + } + + busy = 1; + + iframe->state = NGHTTP2_IB_READ_NBYTE; + inbound_frame_set_mark(iframe, 2); + + break; break; default: busy = 1; diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h index 3e4c1440..99f3de4c 100644 --- a/lib/nghttp2_session.h +++ b/lib/nghttp2_session.h @@ -61,7 +61,8 @@ typedef enum { */ typedef enum { NGHTTP2_TYPEMASK_NONE = 0, - NGHTTP2_TYPEMASK_ALTSVC = 1 << 0 + NGHTTP2_TYPEMASK_ALTSVC = 1 << 0, + NGHTTP2_TYPEMASK_ORIGIN = 2 << 0 } nghttp2_typemask; typedef enum { diff --git a/lib/nghttp2_submit.c b/lib/nghttp2_submit.c index 6c15c824..3510b2e0 100644 --- a/lib/nghttp2_submit.c +++ b/lib/nghttp2_submit.c @@ -486,6 +486,76 @@ int nghttp2_session_set_local_window_size(nghttp2_session *session, return 0; } +int nghttp2_submit_origin(nghttp2_session *session, uint8_t flags, + const uint8_t *origin, size_t origin_len) { + nghttp2_mem *mem; + uint8_t *buf, *p; + uint8_t *origin_copy; + nghttp2_outbound_item *item; + nghttp2_frame *frame; + nghttp2_ext_origin *origin_frame; + int rv; + (void)flags; + + mem = &session->mem; + + if (!session->server) { + return NGHTTP2_ERR_INVALID_STATE; + } + + if (2 + origin_len > NGHTTP2_MAX_PAYLOADLEN) { + return NGHTTP2_ERR_INVALID_ARGUMENT; + } + + buf = nghttp2_mem_malloc(mem, origin_len + 2); + if (buf == NULL) { + return NGHTTP2_ERR_NOMEM; + } + + p = buf; + + origin_copy = p; + if (origin_len) { + p = nghttp2_cpymem(p, origin, origin_len); + } + *p++ = '\0'; + + item = nghttp2_mem_malloc(mem, sizeof(nghttp2_outbound_item)); + if (item == NULL) { + rv = NGHTTP2_ERR_NOMEM; + goto fail_item_malloc; + } + + nghttp2_outbound_item_init(item); + + item->aux_data.ext.builtin = 1; + + origin_frame = &item->ext_frame_payload.origin; + + frame = &item->frame; + frame->ext.payload = origin_frame; + // + // nghttp2_frame_altsvc_init(&frame->ext, stream_id, origin_copy, origin_len, + // field_value_copy, field_value_len); + + nghttp2_frame_origin_init(&frame->ext, origin_copy, origin_len); + + rv = nghttp2_session_add_item(session, item); + if (rv != 0) { + nghttp2_frame_altsvc_free(&frame->ext, mem); + nghttp2_mem_free(mem, item); + + return rv; + } + + return 0; + +fail_item_malloc: + free(buf); + + return rv; +} + int nghttp2_submit_altsvc(nghttp2_session *session, uint8_t flags, int32_t stream_id, const uint8_t *origin, size_t origin_len, const uint8_t *field_value, From 0d324bb6198e7f0ecde05849c56ba0b0ada074f6 Mon Sep 17 00:00:00 2001 From: Lucas Pardue Date: Mon, 24 Apr 2017 18:44:52 +0100 Subject: [PATCH 2/7] Flesh our packing/unpacking --- lib/nghttp2_frame.c | 65 +++++++++++++++++++++ lib/nghttp2_frame.h | 39 +++++++++++++ lib/nghttp2_session.c | 133 ++++++++++++++++++++++++++++++++++++++++++ lib/nghttp2_session.h | 16 ++++- 4 files changed, 252 insertions(+), 1 deletion(-) diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c index 84bdb692..271faadb 100644 --- a/lib/nghttp2_frame.c +++ b/lib/nghttp2_frame.c @@ -760,6 +760,71 @@ int nghttp2_frame_unpack_altsvc_payload2(nghttp2_extension *frame, return 0; } +int nghttp2_frame_pack_origin(nghttp2_bufs *bufs, nghttp2_extension *frame) { + int rv; + nghttp2_buf *buf; + nghttp2_ext_origin *origin_frame; + + origin_frame = frame->payload; + + buf = &bufs->head->buf; + + assert(nghttp2_buf_avail(buf) >= 2 + origin_frame->origin_len); + + buf->pos -= NGHTTP2_FRAME_HDLEN; + + nghttp2_frame_pack_frame_hd(buf->pos, &frame->hd); + + nghttp2_put_uint16be(buf->last, (uint16_t)origin_frame->origin_len); + buf->last += 2; + + rv = nghttp2_bufs_add(bufs, origin_frame->origin, origin_frame->origin_len); + + assert(rv == 0); + + return 0; +} + +void nghttp2_frame_unpack_origin_payload(nghttp2_extension *frame, + size_t origin_len, uint8_t *payload, + size_t payloadlen) { + nghttp2_ext_origin *origin_frame; + uint8_t *p; + + origin_frame = frame->payload; + p = payload; + + origin_frame->origin = p; + + p += origin_len; + + origin_frame->origin_len = origin_len; +} + +int nghttp2_frame_unpack_origin_payload2(nghttp2_extension *frame, + const uint8_t *payload, + size_t payloadlen, nghttp2_mem *mem) { + uint8_t *buf; + size_t origin_len; + + if (payloadlen < 2) { + return NGHTTP2_FRAME_SIZE_ERROR; + } + + origin_len = nghttp2_get_uint16(payload); + + buf = nghttp2_mem_malloc(mem, payloadlen - 2); + if (!buf) { + return NGHTTP2_ERR_NOMEM; + } + + nghttp2_cpymem(buf, payload + 2, payloadlen - 2); + + nghttp2_frame_unpack_origin_payload(frame, origin_len, buf, payloadlen - 2); + + return 0; +} + nghttp2_settings_entry *nghttp2_frame_iv_copy(const nghttp2_settings_entry *iv, size_t niv, nghttp2_mem *mem) { nghttp2_settings_entry *iv_copy; diff --git a/lib/nghttp2_frame.h b/lib/nghttp2_frame.h index d15d3890..7e68faf2 100644 --- a/lib/nghttp2_frame.h +++ b/lib/nghttp2_frame.h @@ -393,6 +393,45 @@ int nghttp2_frame_unpack_altsvc_payload2(nghttp2_extension *frame, const uint8_t *payload, size_t payloadlen, nghttp2_mem *mem); +/* + * Packs ORIGIN frame |frame| in wire frame format and store it in + * |bufs|. + * + * The caller must make sure that nghttp2_bufs_reset(bufs) is called + * before calling this function. + * + * This function always succeeds and returns 0. + */ +int nghttp2_frame_pack_origin(nghttp2_bufs *bufs, nghttp2_extension *ext); + +/* + * Unpacks ORIGIN wire format into |frame|. The |payload| of + * |payloadlen| bytes contains frame payload. This function assumes + * that frame->payload points to the nghttp2_ext_origin object. + * + * This function always succeeds and returns 0. + */ +void nghttp2_frame_unpack_origin_payload(nghttp2_extension *frame, + size_t origin_len, uint8_t *payload, + size_t payloadlen); + +/* + * Unpacks ORIGIN wire format into |frame|. This function only exists + * for unit test. After allocating buffer for fields, this function + * internally calls nghttp2_frame_unpack_origin_payload(). + * + * This function returns 0 if it succeeds, or one of the following + * negative error codes: + * + * NGHTTP2_ERR_NOMEM + * Out of memory. + * NGHTTP2_ERR_FRAME_SIZE_ERROR + * The payload is too small. + */ +int nghttp2_frame_unpack_origin_payload2(nghttp2_extension *frame, + const uint8_t *payload, + size_t payloadlen, nghttp2_mem *mem); + /* * Initializes HEADERS frame |frame| with given values. |frame| takes * ownership of |nva|, so caller must not free it. If |stream_id| is diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 7810ae08..1a65ff47 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -1748,6 +1748,29 @@ static int session_predicate_altsvc_send(nghttp2_session *session, return 0; } +static int session_predicate_origin_send(nghttp2_session *session, + int32_t stream_id) { + nghttp2_stream *stream; + + if (session_is_closing(session)) { + return NGHTTP2_ERR_SESSION_CLOSING; + } + + if (stream_id != 0) { + return 0; + } + + stream = nghttp2_session_get_stream(session, stream_id); + if (stream == NULL) { + return NGHTTP2_ERR_STREAM_CLOSED; + } + if (stream->state == NGHTTP2_STREAM_CLOSING) { + return NGHTTP2_ERR_STREAM_CLOSING; + } + + return 0; +} + /* Take into account settings max frame size and both connection-level flow control here */ static ssize_t @@ -2286,6 +2309,15 @@ static int session_prep_frame(nghttp2_session *session, nghttp2_frame_pack_altsvc(&session->aob.framebufs, &frame->ext); + return 0; + case NGHTTP2_ORIGIN: + rv = session_predicate_origin_send(session, frame->hd.stream_id); + if (rv != 0) { + return rv; + } + + nghttp2_frame_pack_origin(&session->aob.framebufs, &frame->ext); + return 0; default: /* Unreachable here */ @@ -4813,6 +4845,46 @@ static int session_process_altsvc_frame(nghttp2_session *session) { return nghttp2_session_on_altsvc_received(session, frame); } +int nghttp2_session_on_origin_received(nghttp2_session *session, + nghttp2_frame *frame) { + nghttp2_ext_origin *origin_frame; + nghttp2_stream *stream; + + origin_frame = frame->ext.payload; + + /* session->server case has been excluded */ + + if (frame->hd.stream_id == 0) { + return 0; + } else { + stream = nghttp2_session_get_stream(session, frame->hd.stream_id); + if (!stream) { + return 0; + } + + if (stream->state == NGHTTP2_STREAM_CLOSING) { + return 0; + } + } + + return session_call_on_frame_received(session, frame); +} + +static int session_process_origin_frame(nghttp2_session *session) { + nghttp2_inbound_frame *iframe = &session->iframe; + nghttp2_frame *frame = &iframe->frame; + + nghttp2_frame_unpack_origin_payload( + &frame->ext, nghttp2_get_uint16(iframe->sbuf.pos), iframe->lbuf.pos, + nghttp2_buf_len(&iframe->lbuf)); + + /* nghttp2_frame_unpack_altsvc_payload steals buffer from + iframe->lbuf */ + nghttp2_buf_wrap_init(&iframe->lbuf, NULL, 0); + + return nghttp2_session_on_origin_received(session, frame); +} + static int session_process_extension_frame(nghttp2_session *session) { int rv; nghttp2_inbound_frame *iframe = &session->iframe; @@ -6011,6 +6083,37 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in, break; } + case NGHTTP2_ORIGIN: { + size_t origin_len; + + origin_len = nghttp2_get_uint16(iframe->sbuf.pos); + + DEBUGF("recv: origin_len=%zu\n", origin_len); + + if (2 + origin_len > iframe->payloadleft) { + busy = 1; + iframe->state = NGHTTP2_IB_FRAME_SIZE_ERROR; + break; + } + + if (iframe->frame.hd.length > 2) { + iframe->raw_lbuf = + nghttp2_mem_malloc(mem, iframe->frame.hd.length - 2); + + if (iframe->raw_lbuf == NULL) { + return NGHTTP2_ERR_NOMEM; + } + + nghttp2_buf_wrap_init(&iframe->lbuf, iframe->raw_lbuf, + iframe->frame.hd.length); + } + + busy = 1; + + iframe->state = NGHTTP2_IB_READ_ORIGIN_PAYLOAD; + + break; + } default: /* This is unknown frame */ session_inbound_frame_reset(session); @@ -6563,6 +6666,36 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in, session_inbound_frame_reset(session); + break; + case NGHTTP2_IB_READ_ORIGIN_PAYLOAD: + DEBUGF("recv: [IB_READ_ORIGIN_PAYLOAD]\n"); + + readlen = inbound_frame_payload_readlen(iframe, in, last); + + if (readlen > 0) { + iframe->lbuf.last = nghttp2_cpymem(iframe->lbuf.last, in, readlen); + + iframe->payloadleft -= readlen; + in += readlen; + } + + DEBUGF("recv: readlen=%zu, payloadleft=%zu\n", readlen, + iframe->payloadleft); + + if (iframe->payloadleft) { + assert(nghttp2_buf_avail(&iframe->lbuf) > 0); + + break; + } + + rv = session_process_origin_frame(session); + + if (nghttp2_is_fatal(rv)) { + return rv; + } + + session_inbound_frame_reset(session); + break; } diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h index 99f3de4c..9442d9c8 100644 --- a/lib/nghttp2_session.h +++ b/lib/nghttp2_session.h @@ -122,6 +122,7 @@ typedef enum { NGHTTP2_IB_IGN_DATA, NGHTTP2_IB_IGN_ALL, NGHTTP2_IB_READ_ALTSVC_PAYLOAD, + NGHTTP2_IB_READ_ORIGIN_PAYLOAD, NGHTTP2_IB_READ_EXTENSION_PAYLOAD } nghttp2_inbound_state; @@ -738,7 +739,7 @@ int nghttp2_session_on_window_update_received(nghttp2_session *session, nghttp2_frame *frame); /* - * Called when ALTSVC is recieved, assuming |frame| is properly + * Called when ALTSVC is received, assuming |frame| is properly * initialized. * * This function returns 0 if it succeeds, or one of the following @@ -750,6 +751,19 @@ int nghttp2_session_on_window_update_received(nghttp2_session *session, int nghttp2_session_on_altsvc_received(nghttp2_session *session, nghttp2_frame *frame); +/* + * Called when ORIGIN is received, assuming |frame| is properly + * initialized. + * + * This function returns 0 if it succeeds, or one of the following + * negative error codes: + * + * NGHTTP2_ERR_CALLBACK_FAILURE + * The callback function failed. + */ +int nghttp2_session_on_origin_received(nghttp2_session *session, + nghttp2_frame *frame); + /* * Called when DATA is received, assuming |frame| is properly * initialized. From 44e7e3bb40154257f75eede31f33f8a067af893c Mon Sep 17 00:00:00 2001 From: Lucas Pardue Date: Tue, 25 Apr 2017 15:48:52 +0100 Subject: [PATCH 3/7] Add tests. Move around code a little. --- lib/nghttp2_frame.c | 40 +++---- lib/nghttp2_outbound_item.c | 3 + lib/nghttp2_session.c | 2 +- lib/nghttp2_submit.c | 137 +++++++++++----------- src/app_helper.cc | 10 +- tests/main.c | 7 ++ tests/nghttp2_frame_test.c | 47 ++++++++ tests/nghttp2_session_test.c | 213 +++++++++++++++++++++++++++++++++++ tests/nghttp2_test_helper.c | 4 + 9 files changed, 371 insertions(+), 92 deletions(-) diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c index 271faadb..eda076ef 100644 --- a/lib/nghttp2_frame.c +++ b/lib/nghttp2_frame.c @@ -196,26 +196,6 @@ void nghttp2_frame_extension_init(nghttp2_extension *frame, uint8_t type, void nghttp2_frame_extension_free(nghttp2_extension *frame) { (void)frame; } -void nghttp2_frame_origin_init(nghttp2_extension *frame, uint8_t *origin, - size_t origin_len) { - nghttp2_ext_origin *origin_frame; - - /* Always send ORIGIN frame on stream 0 */ - nghttp2_frame_hd_init(&frame->hd, 2 + origin_len, NGHTTP2_ORIGIN, - NGHTTP2_FLAG_NONE, 0); - - origin_frame = frame->payload; - origin_frame->origin = origin; - origin_frame->origin_len = origin_len; -} - -void nghttp2_frame_origin_free(nghttp2_extension *frame, nghttp2_mem *mem) { - nghttp2_ext_origin *origin_frame; - - origin_frame = frame->payload; - nghttp2_mem_free(mem, origin_frame->origin); -} - void nghttp2_frame_altsvc_init(nghttp2_extension *frame, int32_t stream_id, uint8_t *origin, size_t origin_len, uint8_t *field_value, size_t field_value_len) { @@ -248,6 +228,26 @@ size_t nghttp2_frame_priority_len(uint8_t flags) { return 0; } +void nghttp2_frame_origin_init(nghttp2_extension *frame, uint8_t *origin, + size_t origin_len) { + nghttp2_ext_origin *origin_frame; + + /* Always send ORIGIN frame on stream 0 */ + nghttp2_frame_hd_init(&frame->hd, 2 + origin_len, NGHTTP2_ORIGIN, + NGHTTP2_FLAG_NONE, 0); + + origin_frame = frame->payload; + origin_frame->origin = origin; + origin_frame->origin_len = origin_len; +} + +void nghttp2_frame_origin_free(nghttp2_extension *frame, nghttp2_mem *mem) { + nghttp2_ext_origin *origin_frame; + + origin_frame = frame->payload; + nghttp2_mem_free(mem, origin_frame->origin); +} + size_t nghttp2_frame_headers_payload_nv_offset(nghttp2_headers *frame) { return nghttp2_frame_priority_len(frame->hd.flags); } diff --git a/lib/nghttp2_outbound_item.c b/lib/nghttp2_outbound_item.c index 1633cc36..f651c802 100644 --- a/lib/nghttp2_outbound_item.c +++ b/lib/nghttp2_outbound_item.c @@ -86,6 +86,9 @@ void nghttp2_outbound_item_free(nghttp2_outbound_item *item, nghttp2_mem *mem) { case NGHTTP2_ALTSVC: nghttp2_frame_altsvc_free(&frame->ext, mem); break; + case NGHTTP2_ORIGIN: + nghttp2_frame_origin_free(&frame->ext, mem); + break; default: assert(0); break; diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 1a65ff47..cb5a99ea 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -4878,7 +4878,7 @@ static int session_process_origin_frame(nghttp2_session *session) { &frame->ext, nghttp2_get_uint16(iframe->sbuf.pos), iframe->lbuf.pos, nghttp2_buf_len(&iframe->lbuf)); - /* nghttp2_frame_unpack_altsvc_payload steals buffer from + /* nghttp2_frame_unpack_origin_payload steals buffer from iframe->lbuf */ nghttp2_buf_wrap_init(&iframe->lbuf, NULL, 0); diff --git a/lib/nghttp2_submit.c b/lib/nghttp2_submit.c index 3510b2e0..b8699d34 100644 --- a/lib/nghttp2_submit.c +++ b/lib/nghttp2_submit.c @@ -486,76 +486,6 @@ int nghttp2_session_set_local_window_size(nghttp2_session *session, return 0; } -int nghttp2_submit_origin(nghttp2_session *session, uint8_t flags, - const uint8_t *origin, size_t origin_len) { - nghttp2_mem *mem; - uint8_t *buf, *p; - uint8_t *origin_copy; - nghttp2_outbound_item *item; - nghttp2_frame *frame; - nghttp2_ext_origin *origin_frame; - int rv; - (void)flags; - - mem = &session->mem; - - if (!session->server) { - return NGHTTP2_ERR_INVALID_STATE; - } - - if (2 + origin_len > NGHTTP2_MAX_PAYLOADLEN) { - return NGHTTP2_ERR_INVALID_ARGUMENT; - } - - buf = nghttp2_mem_malloc(mem, origin_len + 2); - if (buf == NULL) { - return NGHTTP2_ERR_NOMEM; - } - - p = buf; - - origin_copy = p; - if (origin_len) { - p = nghttp2_cpymem(p, origin, origin_len); - } - *p++ = '\0'; - - item = nghttp2_mem_malloc(mem, sizeof(nghttp2_outbound_item)); - if (item == NULL) { - rv = NGHTTP2_ERR_NOMEM; - goto fail_item_malloc; - } - - nghttp2_outbound_item_init(item); - - item->aux_data.ext.builtin = 1; - - origin_frame = &item->ext_frame_payload.origin; - - frame = &item->frame; - frame->ext.payload = origin_frame; - // - // nghttp2_frame_altsvc_init(&frame->ext, stream_id, origin_copy, origin_len, - // field_value_copy, field_value_len); - - nghttp2_frame_origin_init(&frame->ext, origin_copy, origin_len); - - rv = nghttp2_session_add_item(session, item); - if (rv != 0) { - nghttp2_frame_altsvc_free(&frame->ext, mem); - nghttp2_mem_free(mem, item); - - return rv; - } - - return 0; - -fail_item_malloc: - free(buf); - - return rv; -} - int nghttp2_submit_altsvc(nghttp2_session *session, uint8_t flags, int32_t stream_id, const uint8_t *origin, size_t origin_len, const uint8_t *field_value, @@ -641,6 +571,73 @@ fail_item_malloc: return rv; } +int nghttp2_submit_origin(nghttp2_session *session, uint8_t flags, + const uint8_t *origin, size_t origin_len) { + nghttp2_mem *mem; + uint8_t *buf, *p; + uint8_t *origin_copy; + nghttp2_outbound_item *item; + nghttp2_frame *frame; + nghttp2_ext_origin *origin_frame; + int rv; + (void)flags; + + mem = &session->mem; + + if (!session->server) { + return NGHTTP2_ERR_INVALID_STATE; + } + + if (2 + origin_len > NGHTTP2_MAX_PAYLOADLEN) { + return NGHTTP2_ERR_INVALID_ARGUMENT; + } + + buf = nghttp2_mem_malloc(mem, origin_len + 2); + if (buf == NULL) { + return NGHTTP2_ERR_NOMEM; + } + + p = buf; + + origin_copy = p; + if (origin_len) { + p = nghttp2_cpymem(p, origin, origin_len); + } + *p++ = '\0'; + + item = nghttp2_mem_malloc(mem, sizeof(nghttp2_outbound_item)); + if (item == NULL) { + rv = NGHTTP2_ERR_NOMEM; + goto fail_item_malloc; + } + + nghttp2_outbound_item_init(item); + + item->aux_data.ext.builtin = 1; + + origin_frame = &item->ext_frame_payload.origin; + + frame = &item->frame; + frame->ext.payload = origin_frame; + + nghttp2_frame_origin_init(&frame->ext, origin_copy, origin_len); + + rv = nghttp2_session_add_item(session, item); + if (rv != 0) { + nghttp2_frame_altsvc_free(&frame->ext, mem); + nghttp2_mem_free(mem, item); + + return rv; + } + + return 0; + +fail_item_malloc: + free(buf); + + return rv; +} + static uint8_t set_request_flags(const nghttp2_priority_spec *pri_spec, const nghttp2_data_provider *data_prd) { uint8_t flags = NGHTTP2_FLAG_NONE; diff --git a/src/app_helper.cc b/src/app_helper.cc index a736d308..c5880a8e 100644 --- a/src/app_helper.cc +++ b/src/app_helper.cc @@ -106,6 +106,8 @@ std::string strframetype(uint8_t type) { return "WINDOW_UPDATE"; case NGHTTP2_ALTSVC: return "ALTSVC"; + case NGHTTP2_ORIGIN: + return "ORIGIN"; } std::string s = "extension(0x"; @@ -350,10 +352,16 @@ void print_frame(print_type ptype, const nghttp2_frame *frame) { static_cast(altsvc->field_value_len), altsvc->field_value); break; } + case NGHTTP2_ORIGIN: { + auto origin_frame = static_cast(frame->ext.payload); + print_frame_attr_indent(); + fprintf(outfile, "(origin=[%.*s])\n", + static_cast(origin_frame->origin_len), origin_frame->origin); + break; + } default: break; } -} } // namespace int verbose_on_header_callback(nghttp2_session *session, diff --git a/tests/main.c b/tests/main.c index a922da6e..7ae4ebfa 100644 --- a/tests/main.c +++ b/tests/main.c @@ -105,6 +105,8 @@ int main() { test_nghttp2_session_recv_extension) || !CU_add_test(pSuite, "session_recv_altsvc", test_nghttp2_session_recv_altsvc) || + !CU_add_test(pSuite, "session_recv_origin", + test_nghttp2_session_recv_origin) || !CU_add_test(pSuite, "session_continue", test_nghttp2_session_continue) || !CU_add_test(pSuite, "session_add_frame", test_nghttp2_session_add_frame) || @@ -136,6 +138,8 @@ int main() { test_nghttp2_session_on_data_received_fail_fast) || !CU_add_test(pSuite, "session_on_altsvc_received", test_nghttp2_session_on_altsvc_received) || + !CU_add_test(pSuite, "session_on_origin_received", + test_nghttp2_session_on_origin_received) || !CU_add_test(pSuite, "session_send_headers_start_stream", test_nghttp2_session_send_headers_start_stream) || !CU_add_test(pSuite, "session_send_headers_reply", @@ -204,6 +208,7 @@ int main() { test_nghttp2_submit_invalid_nv) || !CU_add_test(pSuite, "submit_extension", test_nghttp2_submit_extension) || !CU_add_test(pSuite, "submit_altsvc", test_nghttp2_submit_altsvc) || + !CU_add_test(pSuite, "submit_origin", test_nghttp2_submit_origin) || !CU_add_test(pSuite, "session_open_stream", test_nghttp2_session_open_stream) || !CU_add_test(pSuite, "session_open_stream_with_idle_stream_dep", @@ -353,6 +358,8 @@ int main() { test_nghttp2_frame_pack_window_update) || !CU_add_test(pSuite, "frame_pack_altsvc", test_nghttp2_frame_pack_altsvc) || + !CU_add_test(pSuite, "frame_pack_origin", + test_nghttp2_frame_pack_origin) || !CU_add_test(pSuite, "nv_array_copy", test_nghttp2_nv_array_copy) || !CU_add_test(pSuite, "iv_check", test_nghttp2_iv_check) || !CU_add_test(pSuite, "hd_deflate", test_nghttp2_hd_deflate) || diff --git a/tests/nghttp2_frame_test.c b/tests/nghttp2_frame_test.c index e1064e03..931c91be 100644 --- a/tests/nghttp2_frame_test.c +++ b/tests/nghttp2_frame_test.c @@ -508,6 +508,53 @@ void test_nghttp2_frame_pack_altsvc(void) { nghttp2_bufs_free(&bufs); } +void test_nghttp2_frame_pack_origin(void) { + nghttp2_extension frame, oframe; + nghttp2_ext_origin origin_frame, oorigin_frame; + nghttp2_bufs bufs; + int rv; + size_t payloadlen; + static const uint8_t origin[] = "www.nghttp2.org"; + nghttp2_buf buf; + uint8_t *rawbuf; + nghttp2_mem *mem; + + mem = nghttp2_mem_default(); + + frame_pack_bufs_init(&bufs); + + frame.payload = &origin_frame; + oframe.payload = &oorigin_frame; + + rawbuf = nghttp2_mem_malloc(mem, 32); + nghttp2_buf_wrap_init(&buf, rawbuf, 32); + + buf.last = nghttp2_cpymem(buf.last, origin, sizeof(origin) - 1); + + nghttp2_frame_origin_init(&frame, buf.pos, sizeof(origin) - 1); + + payloadlen = 2 + sizeof(origin) - 1; + + rv = nghttp2_frame_pack_origin(&bufs, &frame); + + CU_ASSERT(0 == rv); + CU_ASSERT(NGHTTP2_FRAME_HDLEN + payloadlen == nghttp2_bufs_len(&bufs)); + + rv = unpack_framebuf((nghttp2_frame *)&oframe, &bufs); + + CU_ASSERT(0 == rv); + + check_frame_header(payloadlen, NGHTTP2_ORIGIN, NGHTTP2_FLAG_NONE, 0, + &oframe.hd); + + CU_ASSERT(sizeof(origin) - 1 == oorigin_frame.origin_len); + CU_ASSERT(0 == memcmp(origin, oorigin_frame.origin, sizeof(origin) - 1)); + + nghttp2_frame_origin_free(&oframe, mem); + nghttp2_frame_origin_free(&frame, mem); + nghttp2_bufs_free(&bufs); +} + void test_nghttp2_nv_array_copy(void) { nghttp2_nv *nva; ssize_t rv; diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index d828028a..9f58aeb2 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -2254,6 +2254,121 @@ void test_nghttp2_session_recv_altsvc(void) { nghttp2_option_del(option); } +void test_nghttp2_session_recv_origin(void) { + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + my_user_data ud; + nghttp2_buf buf; + nghttp2_frame_hd hd; + nghttp2_mem *mem; + ssize_t rv; + nghttp2_option *option; + static const uint8_t origin[] = "www.nghttp2.org"; + + mem = nghttp2_mem_default(); + + nghttp2_buf_init2(&buf, NGHTTP2_FRAME_HDLEN + NGHTTP2_MAX_FRAME_SIZE_MIN, + mem); + + memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); + + callbacks.on_frame_recv_callback = on_frame_recv_callback; + + nghttp2_option_new(&option); + nghttp2_option_set_builtin_recv_extension_type(option, NGHTTP2_ORIGIN); + + nghttp2_session_client_new2(&session, &callbacks, &ud, option); + + nghttp2_frame_hd_init(&hd, 2 + sizeof(origin) - 1, NGHTTP2_ORIGIN, + NGHTTP2_FLAG_NONE, 0); + nghttp2_frame_pack_frame_hd(buf.last, &hd); + buf.last += NGHTTP2_FRAME_HDLEN; + nghttp2_put_uint16be(buf.last, sizeof(origin) - 1); + buf.last += 2; + buf.last = nghttp2_cpymem(buf.last, origin, sizeof(origin) - 1); + + ud.frame_recv_cb_called = 0; + rv = nghttp2_session_mem_recv(session, buf.pos, nghttp2_buf_len(&buf)); + + CU_ASSERT((ssize_t)nghttp2_buf_len(&buf) == rv); + CU_ASSERT(1 == ud.frame_recv_cb_called); + CU_ASSERT(NGHTTP2_ORIGIN == ud.recv_frame_hd.type); + CU_ASSERT(NGHTTP2_FLAG_NONE == ud.recv_frame_hd.flags); + CU_ASSERT(0 == ud.recv_frame_hd.stream_id); + + nghttp2_session_del(session); + + /* size of origin is larger than frame length */ + nghttp2_buf_reset(&buf); + + nghttp2_session_client_new2(&session, &callbacks, &ud, option); + + nghttp2_frame_hd_init(&hd, 2 + sizeof(origin) - 1 - 1, NGHTTP2_ORIGIN, + NGHTTP2_FLAG_NONE, 0); + nghttp2_frame_pack_frame_hd(buf.last, &hd); + buf.last += NGHTTP2_FRAME_HDLEN; + nghttp2_put_uint16be(buf.last, sizeof(origin) - 1); + buf.last += 2; + buf.last = nghttp2_cpymem(buf.last, origin, sizeof(origin) - 1 - 1); + + ud.frame_recv_cb_called = 0; + rv = nghttp2_session_mem_recv(session, buf.pos, nghttp2_buf_len(&buf)); + + CU_ASSERT((ssize_t)nghttp2_buf_len(&buf) == rv); + CU_ASSERT(0 == ud.frame_recv_cb_called); + + nghttp2_session_del(session); + + /* send large frame (16KiB) */ + nghttp2_buf_reset(&buf); + + nghttp2_session_client_new2(&session, &callbacks, &ud, option); + + nghttp2_frame_hd_init(&hd, NGHTTP2_MAX_FRAME_SIZE_MIN, NGHTTP2_ORIGIN, + NGHTTP2_FLAG_NONE, 0); + nghttp2_frame_pack_frame_hd(buf.last, &hd); + buf.last += NGHTTP2_FRAME_HDLEN; + nghttp2_put_uint16be(buf.last, sizeof(origin) - 1); + buf.last += 2; + buf.last = nghttp2_cpymem(buf.last, origin, sizeof(origin) - 1); + memset(buf.last, 0, nghttp2_buf_avail(&buf)); + buf.last += nghttp2_buf_avail(&buf); + + ud.frame_recv_cb_called = 0; + rv = nghttp2_session_mem_recv(session, buf.pos, nghttp2_buf_len(&buf)); + + CU_ASSERT((ssize_t)nghttp2_buf_len(&buf) == rv); + CU_ASSERT(1 == ud.frame_recv_cb_called); + CU_ASSERT(NGHTTP2_ORIGIN == ud.recv_frame_hd.type); + CU_ASSERT(NGHTTP2_MAX_FRAME_SIZE_MIN == ud.recv_frame_hd.length); + + nghttp2_session_del(session); + + /* received by server */ + nghttp2_buf_reset(&buf); + + nghttp2_session_server_new2(&session, &callbacks, &ud, option); + + nghttp2_frame_hd_init(&hd, 2 + sizeof(origin) - 1, NGHTTP2_ORIGIN, + NGHTTP2_FLAG_NONE, 0); + nghttp2_frame_pack_frame_hd(buf.last, &hd); + buf.last += NGHTTP2_FRAME_HDLEN; + nghttp2_put_uint16be(buf.last, sizeof(origin) - 1); + buf.last += 2; + buf.last = nghttp2_cpymem(buf.last, origin, sizeof(origin) - 1); + + ud.frame_recv_cb_called = 0; + rv = nghttp2_session_mem_recv(session, buf.pos, nghttp2_buf_len(&buf)); + + CU_ASSERT((ssize_t)nghttp2_buf_len(&buf) == rv); + CU_ASSERT(0 == ud.frame_recv_cb_called); + + nghttp2_session_del(session); + + nghttp2_buf_free(&buf, mem); + nghttp2_option_del(option); +} + void test_nghttp2_session_continue(void) { nghttp2_session *session; nghttp2_session_callbacks callbacks; @@ -3767,6 +3882,55 @@ void test_nghttp2_session_on_altsvc_received(void) { nghttp2_option_del(option); } +void test_nghttp2_session_on_origin_received(void) { + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + my_user_data ud; + nghttp2_frame frame; + nghttp2_option *option; + uint8_t origin[] = "www.nghttp2.org"; + int rv; + + memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); + callbacks.on_frame_recv_callback = on_frame_recv_callback; + + nghttp2_option_new(&option); + nghttp2_option_set_builtin_recv_extension_type(option, NGHTTP2_ORIGIN); + + nghttp2_session_client_new2(&session, &callbacks, &ud, option); + + frame.ext.payload = &session->iframe.ext_frame_payload; + + /* We just pass the strings without making a copy. This is OK, + since we never call nghttp2_frame_origin_free(). */ + nghttp2_frame_origin_init(&frame.ext, origin, sizeof(origin) - 1); + + ud.frame_recv_cb_called = 0; + rv = nghttp2_session_on_origin_received(session, &frame); + + CU_ASSERT(0 == rv); + CU_ASSERT(1 == ud.frame_recv_cb_called); + + nghttp2_session_del(session); + + /* Receiving empty origin; this is OK */ + nghttp2_session_client_new2(&session, &callbacks, &ud, option); + + frame.ext.payload = &session->iframe.ext_frame_payload; + + nghttp2_frame_origin_init(&frame.ext, origin, 0); + + ud.frame_recv_cb_called = 0; + rv = nghttp2_session_on_origin_received(session, &frame); + + CU_ASSERT(0 == rv); + CU_ASSERT(1 == ud.frame_recv_cb_called); + + nghttp2_session_del(session); + + nghttp2_option_del(option); +} + void test_nghttp2_session_send_headers_start_stream(void) { nghttp2_session *session; nghttp2_session_callbacks callbacks; @@ -5855,6 +6019,55 @@ void test_nghttp2_submit_altsvc(void) { nghttp2_session_del(session); } +void test_nghttp2_submit_origin(void) { + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + my_user_data ud; + int rv; + ssize_t len; + const uint8_t *data; + nghttp2_frame_hd hd; + size_t origin_len; + const uint8_t origin[] = "www.nghttp2.org"; + + memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); + + nghttp2_session_server_new(&session, &callbacks, &ud); + + rv = nghttp2_submit_origin(session, NGHTTP2_FLAG_NONE, origin, + sizeof(origin) - 1); + + CU_ASSERT(0 == rv); + + ud.frame_send_cb_called = 0; + + len = nghttp2_session_mem_send(session, &data); + + CU_ASSERT(len == NGHTTP2_FRAME_HDLEN + 2 + sizeof(origin) - 1); + + nghttp2_frame_unpack_frame_hd(&hd, data); + + CU_ASSERT(2 + sizeof(origin) - 1 == hd.length); + CU_ASSERT(NGHTTP2_ORIGIN == hd.type); + CU_ASSERT(NGHTTP2_FLAG_NONE == hd.flags); + + origin_len = nghttp2_get_uint16(data + NGHTTP2_FRAME_HDLEN); + + CU_ASSERT(sizeof(origin) - 1 == origin_len); + CU_ASSERT(0 == + memcmp(origin, data + NGHTTP2_FRAME_HDLEN + 2, sizeof(origin) - 1)); + + /* submitting from client side session is error */ + nghttp2_session_client_new(&session, &callbacks, NULL); + + rv = nghttp2_submit_origin(session, NGHTTP2_FLAG_NONE, origin, + sizeof(origin) - 1); + + CU_ASSERT(NGHTTP2_ERR_INVALID_STATE == rv); + + nghttp2_session_del(session); +} + void test_nghttp2_session_open_stream(void) { nghttp2_session *session; nghttp2_session_callbacks callbacks; diff --git a/tests/nghttp2_test_helper.c b/tests/nghttp2_test_helper.c index b8869f5c..2c22aaab 100644 --- a/tests/nghttp2_test_helper.c +++ b/tests/nghttp2_test_helper.c @@ -84,6 +84,10 @@ int unpack_frame(nghttp2_frame *frame, const uint8_t *in, size_t len) { assert(payloadlen > 2); nghttp2_frame_unpack_altsvc_payload2(&frame->ext, payload, payloadlen, mem); break; + case NGHTTP2_ORIGIN: + assert(payloadlen > 2); + nghttp2_frame_unpack_origin_payload2(&frame->ext, payload, payloadlen, mem); + break; default: /* Must not be reachable */ assert(0); From a59728da4db80aa22d2827b2835fd0602b9a4b2e Mon Sep 17 00:00:00 2001 From: Lucas Pardue Date: Tue, 25 Apr 2017 15:58:20 +0100 Subject: [PATCH 4/7] Restore deleted } --- src/app_helper.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app_helper.cc b/src/app_helper.cc index c5880a8e..ec51a4de 100644 --- a/src/app_helper.cc +++ b/src/app_helper.cc @@ -362,6 +362,7 @@ void print_frame(print_type ptype, const nghttp2_frame *frame) { default: break; } +} } // namespace int verbose_on_header_callback(nghttp2_session *session, From 86c27488e848f83d26cb7683e4c38de11baa05c4 Mon Sep 17 00:00:00 2001 From: Lucas Pardue Date: Tue, 25 Apr 2017 16:22:14 +0100 Subject: [PATCH 5/7] Address CI build errors/warnings --- lib/nghttp2_frame.c | 9 ++------- lib/nghttp2_frame.h | 1 - lib/nghttp2_session.c | 2 +- tests/nghttp2_session_test.h | 3 +++ 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c index eda076ef..46ed10d1 100644 --- a/lib/nghttp2_frame.c +++ b/lib/nghttp2_frame.c @@ -789,15 +789,10 @@ void nghttp2_frame_unpack_origin_payload(nghttp2_extension *frame, size_t origin_len, uint8_t *payload, size_t payloadlen) { nghttp2_ext_origin *origin_frame; - uint8_t *p; + (void)payloadlen; origin_frame = frame->payload; - p = payload; - - origin_frame->origin = p; - - p += origin_len; - + origin_frame->origin = payload; origin_frame->origin_len = origin_len; } diff --git a/lib/nghttp2_frame.h b/lib/nghttp2_frame.h index 7e68faf2..cf0dd597 100644 --- a/lib/nghttp2_frame.h +++ b/lib/nghttp2_frame.h @@ -431,7 +431,6 @@ void nghttp2_frame_unpack_origin_payload(nghttp2_extension *frame, int nghttp2_frame_unpack_origin_payload2(nghttp2_extension *frame, const uint8_t *payload, size_t payloadlen, nghttp2_mem *mem); - /* * Initializes HEADERS frame |frame| with given values. |frame| takes * ownership of |nva|, so caller must not free it. If |stream_id| is diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index cb5a99ea..9a221564 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -5804,6 +5804,7 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in, iframe->state = NGHTTP2_IB_READ_NBYTE; inbound_frame_set_mark(iframe, 2); + break; case NGHTTP2_ORIGIN: if ((session->builtin_recv_ext_types & NGHTTP2_TYPEMASK_ORIGIN) == 0) { @@ -5835,7 +5836,6 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in, inbound_frame_set_mark(iframe, 2); break; - break; default: busy = 1; diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index 470b274c..60989e2d 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -46,6 +46,7 @@ void test_nghttp2_session_recv_settings_header_table_size(void); void test_nghttp2_session_recv_too_large_frame_length(void); void test_nghttp2_session_recv_extension(void); void test_nghttp2_session_recv_altsvc(void); +void test_nghttp2_session_recv_origin(void); void test_nghttp2_session_continue(void); void test_nghttp2_session_add_frame(void); void test_nghttp2_session_on_request_headers_received(void); @@ -62,6 +63,7 @@ void test_nghttp2_session_on_window_update_received(void); void test_nghttp2_session_on_data_received(void); void test_nghttp2_session_on_data_received_fail_fast(void); void test_nghttp2_session_on_altsvc_received(void); +void test_nghttp2_session_on_origin_received(void); void test_nghttp2_session_send_headers_start_stream(void); void test_nghttp2_session_send_headers_reply(void); void test_nghttp2_session_send_headers_frame_size_error(void); @@ -99,6 +101,7 @@ void test_nghttp2_submit_shutdown_notice(void); void test_nghttp2_submit_invalid_nv(void); void test_nghttp2_submit_extension(void); void test_nghttp2_submit_altsvc(void); +void test_nghttp2_submit_origin(void); void test_nghttp2_session_open_stream(void); void test_nghttp2_session_open_stream_with_idle_stream_dep(void); void test_nghttp2_session_get_next_ob_item(void); From f42c1efd59f8b98905561178dca182506058bf19 Mon Sep 17 00:00:00 2001 From: Lucas Pardue Date: Tue, 25 Apr 2017 16:32:42 +0100 Subject: [PATCH 6/7] Address more CI build errors/warnings --- tests/nghttp2_frame_test.h | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/nghttp2_frame_test.h b/tests/nghttp2_frame_test.h index 059b20ba..8c0d6d5b 100644 --- a/tests/nghttp2_frame_test.h +++ b/tests/nghttp2_frame_test.h @@ -39,6 +39,7 @@ void test_nghttp2_frame_pack_ping(void); void test_nghttp2_frame_pack_goaway(void); void test_nghttp2_frame_pack_window_update(void); void test_nghttp2_frame_pack_altsvc(void); +void test_nghttp2_frame_pack_origin(void); void test_nghttp2_nv_array_copy(void); void test_nghttp2_iv_check(void); From d2c3e1fc4508d88ab2d1f1a99157d4734cc318c8 Mon Sep 17 00:00:00 2001 From: Lucas Pardue Date: Tue, 25 Apr 2017 20:17:47 +0100 Subject: [PATCH 7/7] Fix segfault due to incorrect free, other tweaks --- lib/nghttp2_session.c | 23 ++++------------------- lib/nghttp2_submit.c | 2 +- 2 files changed, 5 insertions(+), 20 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 9a221564..86313ad3 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -1760,14 +1760,6 @@ static int session_predicate_origin_send(nghttp2_session *session, return 0; } - stream = nghttp2_session_get_stream(session, stream_id); - if (stream == NULL) { - return NGHTTP2_ERR_STREAM_CLOSED; - } - if (stream->state == NGHTTP2_STREAM_CLOSING) { - return NGHTTP2_ERR_STREAM_CLOSING; - } - return 0; } @@ -4826,7 +4818,6 @@ int nghttp2_session_on_altsvc_received(nghttp2_session *session, return 0; } } - return session_call_on_frame_received(session, frame); } @@ -4854,17 +4845,8 @@ int nghttp2_session_on_origin_received(nghttp2_session *session, /* session->server case has been excluded */ - if (frame->hd.stream_id == 0) { + if (frame->hd.stream_id != 0) { return 0; - } else { - stream = nghttp2_session_get_stream(session, frame->hd.stream_id); - if (!stream) { - return 0; - } - - if (stream->state == NGHTTP2_STREAM_CLOSING) { - return 0; - } } return session_call_on_frame_received(session, frame); @@ -6090,6 +6072,9 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in, DEBUGF("recv: origin_len=%zu\n", origin_len); + /* + * TODO: figure out why this check fails + */ if (2 + origin_len > iframe->payloadleft) { busy = 1; iframe->state = NGHTTP2_IB_FRAME_SIZE_ERROR; diff --git a/lib/nghttp2_submit.c b/lib/nghttp2_submit.c index b8699d34..e11f5be4 100644 --- a/lib/nghttp2_submit.c +++ b/lib/nghttp2_submit.c @@ -624,7 +624,7 @@ int nghttp2_submit_origin(nghttp2_session *session, uint8_t flags, rv = nghttp2_session_add_item(session, item); if (rv != 0) { - nghttp2_frame_altsvc_free(&frame->ext, mem); + nghttp2_frame_origin_free(&frame->ext, mem); nghttp2_mem_free(mem, item); return rv;