From 2d48b40a1d1b22807bc55965ba4fe263dadcdd30 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Mon, 11 Apr 2016 10:17:28 +0000 Subject: [PATCH] reduced h2_request initialization/copy after review by CJ git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1738563 13f79535-47bb-0310-9956-ffa450edef68 --- modules/http2/h2_io.c | 2 +- modules/http2/h2_request.c | 40 +++++++++++--------------------------- modules/http2/h2_request.h | 2 -- modules/http2/h2_session.c | 13 +++++++------ modules/http2/h2_session.h | 7 ++++++- modules/http2/h2_stream.c | 29 +++++++++++++++------------ modules/http2/h2_stream.h | 12 ++---------- 7 files changed, 44 insertions(+), 61 deletions(-) diff --git a/modules/http2/h2_io.c b/modules/http2/h2_io.c index 5bbf09e99b..6c140a0893 100644 --- a/modules/http2/h2_io.c +++ b/modules/http2/h2_io.c @@ -43,7 +43,7 @@ h2_io *h2_io_create(int id, apr_pool_t *pool, io->id = id; io->pool = pool; io->bucket_alloc = bucket_alloc; - io->request = h2_request_clone(pool, request); + io->request = request; } return io; } diff --git a/modules/http2/h2_request.c b/modules/http2/h2_request.c index 6f8016a363..a12b55072a 100644 --- a/modules/http2/h2_request.c +++ b/modules/http2/h2_request.c @@ -235,7 +235,8 @@ apr_status_t h2_request_end_headers(h2_request *req, apr_pool_t *pool, const char *s; if (req->eoh) { - return APR_EINVAL; + /* already done */ + return APR_SUCCESS; } /* rfc7540, ch. 8.1.2.3: @@ -337,37 +338,18 @@ apr_status_t h2_request_add_trailer(h2_request *req, apr_pool_t *pool, return add_h1_trailer(req, pool, name, nlen, value, vlen); } -#define OPT_COPY(p, s) ((s)? apr_pstrdup(p, s) : NULL) - -void h2_request_copy(apr_pool_t *p, h2_request *dst, const h2_request *src) +h2_request *h2_request_clone(apr_pool_t *p, const h2_request *src) { - /* keep the dst id */ - dst->initiated_on = src->initiated_on; - dst->method = OPT_COPY(p, src->method); - dst->scheme = OPT_COPY(p, src->scheme); - dst->authority = OPT_COPY(p, src->authority); - dst->path = OPT_COPY(p, src->path); - dst->headers = apr_table_clone(p, src->headers); + h2_request *dst = apr_pmemdup(p, src, sizeof(*dst)); + dst->method = apr_pstrdup(p, src->method); + dst->scheme = apr_pstrdup(p, src->scheme); + dst->authority = apr_pstrdup(p, src->authority); + dst->path = apr_pstrdup(p, src->path); + dst->headers = apr_table_clone(p, src->headers); if (src->trailers) { - dst->trailers = apr_table_clone(p, src->trailers); - } - else { - dst->trailers = NULL; + dst->trailers = apr_table_clone(p, src->trailers); } - dst->content_length = src->content_length; - dst->chunked = src->chunked; - dst->eoh = src->eoh; - dst->body = src->body; - dst->serialize = src->serialize; - dst->push_policy = src->push_policy; -} - -h2_request *h2_request_clone(apr_pool_t *p, const h2_request *src) -{ - h2_request *nreq = apr_pcalloc(p, sizeof(*nreq)); - memcpy(nreq, src, sizeof(*nreq)); - h2_request_copy(p, nreq, src); - return nreq; + return dst; } request_rec *h2_request_create_rec(const h2_request *req, conn_rec *c) diff --git a/modules/http2/h2_request.h b/modules/http2/h2_request.h index da87d70a50..4288dfec2c 100644 --- a/modules/http2/h2_request.h +++ b/modules/http2/h2_request.h @@ -43,8 +43,6 @@ apr_status_t h2_request_add_trailer(h2_request *req, apr_pool_t *pool, apr_status_t h2_request_end_headers(h2_request *req, apr_pool_t *pool, int eos, int push); -void h2_request_copy(apr_pool_t *p, h2_request *dst, const h2_request *src); - h2_request *h2_request_clone(apr_pool_t *p, const h2_request *src); /** diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c index 3cfa7b665b..8da2e9f2bf 100644 --- a/modules/http2/h2_session.c +++ b/modules/http2/h2_session.c @@ -121,7 +121,8 @@ static void cleanup_streams(h2_session *session) } } -h2_stream *h2_session_open_stream(h2_session *session, int stream_id) +h2_stream *h2_session_open_stream(h2_session *session, int stream_id, + int initiated_on, const h2_request *req) { h2_stream * stream; apr_pool_t *stream_pool; @@ -135,7 +136,8 @@ h2_stream *h2_session_open_stream(h2_session *session, int stream_id) apr_pool_tag(stream_pool, "h2_stream"); } - stream = h2_stream_open(stream_id, stream_pool, session); + stream = h2_stream_open(stream_id, stream_pool, session, + initiated_on, req); h2_ihash_add(session->streams, stream); if (H2_STREAM_CLIENT_INITIATED(stream_id)) { @@ -360,7 +362,7 @@ static int on_begin_headers_cb(nghttp2_session *ngh2, /* nop */ } else { - s = h2_session_open_stream((h2_session *)userp, frame->hd.stream_id); + s = h2_session_open_stream(userp, frame->hd.stream_id, 0, NULL); } return s? 0 : NGHTTP2_ERR_START_STREAM_NOT_ALLOWED; } @@ -1030,7 +1032,7 @@ static apr_status_t h2_session_start(h2_session *session, int *rv) } /* Now we need to auto-open stream 1 for the request we got. */ - stream = h2_session_open_stream(session, 1); + stream = h2_session_open_stream(session, 1, 0, NULL); if (!stream) { status = APR_EGENERAL; ap_log_rerror(APLOG_MARK, APLOG_ERR, status, session->r, @@ -1372,9 +1374,8 @@ struct h2_stream *h2_session_push(h2_session *session, h2_stream *is, session->id, is->id, nid, push->req->method, push->req->path, is->id); - stream = h2_session_open_stream(session, nid); + stream = h2_session_open_stream(session, nid, is->id, push->req); if (stream) { - h2_stream_set_h2_request(stream, is->id, push->req); status = stream_schedule(session, stream, 1); if (status != APR_SUCCESS) { ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, session->c, diff --git a/modules/http2/h2_session.h b/modules/http2/h2_session.h index ea5f82a3b1..28eedba843 100644 --- a/modules/http2/h2_session.h +++ b/modules/http2/h2_session.h @@ -198,9 +198,14 @@ struct h2_stream *h2_session_get_stream(h2_session *session, int stream_id); * * @param session the session to register in * @param stream_id the new stream identifier + * @param initiated_on the stream id this one is initiated on or 0 + * @param req the request for this stream or NULL if not known yet * @return the new stream */ -struct h2_stream *h2_session_open_stream(h2_session *session, int stream_id); +struct h2_stream *h2_session_open_stream(h2_session *session, int stream_id, + int initiated_on, + const h2_request *req); + /** * Returns if client settings have push enabled. diff --git a/modules/http2/h2_stream.c b/modules/http2/h2_stream.c index 0a1dadf9dd..a7979e3ffa 100644 --- a/modules/http2/h2_stream.c +++ b/modules/http2/h2_stream.c @@ -137,16 +137,29 @@ static int output_open(h2_stream *stream) static h2_sos *h2_sos_mplx_create(h2_stream *stream, h2_response *response); -h2_stream *h2_stream_open(int id, apr_pool_t *pool, h2_session *session) +h2_stream *h2_stream_open(int id, apr_pool_t *pool, h2_session *session, + int initiated_on, const h2_request *creq) { + h2_request *req; h2_stream *stream = apr_pcalloc(pool, sizeof(h2_stream)); + stream->id = id; stream->state = H2_STREAM_ST_IDLE; stream->pool = pool; stream->session = session; set_state(stream, H2_STREAM_ST_OPEN); - stream->request = h2_request_create(id, pool, - h2_config_geti(session->config, H2_CONF_SER_HEADERS)); + + if (creq) { + /* take it into out pool and assure correct id's */ + req = h2_request_clone(pool, creq); + req->id = id; + req->initiated_on = initiated_on; + } + else { + req = h2_request_create(id, pool, + h2_config_geti(session->config, H2_CONF_SER_HEADERS)); + } + stream->request = req; ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, APLOGNO(03082) "h2_stream(%ld-%d): opened", session->id, stream->id); @@ -231,14 +244,6 @@ apr_status_t h2_stream_set_request(h2_stream *stream, request_rec *r) return status; } -void h2_stream_set_h2_request(h2_stream *stream, int initiated_on, - const h2_request *req) -{ - h2_request_copy(stream->pool, stream->request, req); - stream->request->initiated_on = initiated_on; - stream->request->eoh = 0; -} - apr_status_t h2_stream_add_header(h2_stream *stream, const char *name, size_t nlen, const char *value, size_t vlen) @@ -298,7 +303,7 @@ apr_status_t h2_stream_schedule(h2_stream *stream, int eos, int push_enabled, } else { h2_stream_rst(stream, H2_ERR_INTERNAL_ERROR); - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, stream->session->c, + ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, stream->session->c, "h2_stream(%ld-%d): RST=2 (internal err) %s %s://%s%s", stream->session->id, stream->id, stream->request->method, stream->request->scheme, diff --git a/modules/http2/h2_stream.h b/modules/http2/h2_stream.h index f0cd2167a3..6f142d9dc0 100644 --- a/modules/http2/h2_stream.h +++ b/modules/http2/h2_stream.h @@ -71,7 +71,8 @@ struct h2_stream { * @param session the session this stream belongs to * @return the newly opened stream */ -h2_stream *h2_stream_open(int id, apr_pool_t *pool, struct h2_session *session); +h2_stream *h2_stream_open(int id, apr_pool_t *pool, struct h2_session *session, + int initiated_on, const struct h2_request *req); /** * Destroy any resources held by this stream. Will destroy memory pool @@ -106,15 +107,6 @@ apr_pool_t *h2_stream_detach_pool(h2_stream *stream); */ apr_status_t h2_stream_set_request(h2_stream *stream, request_rec *r); -/** - * Initialize stream->request with the given h2_request. - * - * @param stream the stream to init the request for - * @param req the request for initializing, will be copied - */ -void h2_stream_set_h2_request(h2_stream *stream, int initiated_on, - const struct h2_request *req); - /* * Add a HTTP/2 header (including pseudo headers) or trailer * to the given stream, depending on stream state. -- 2.40.0