From 5d6c9e84ab378616c025af816f172fce7aaeba65 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Mon, 24 Aug 2015 16:07:39 +0000 Subject: [PATCH] removed mod_h2 directives which were experimental and should not support any longer, changed H2Direct to only be enabled by default when not using TLS, conf. to rfc7540 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1697446 13f79535-47bb-0310-9956-ffa450edef68 --- docs/manual/mod/mod_h2.xml | 72 +------------------------------------- modules/http2/h2_config.c | 65 +--------------------------------- modules/http2/h2_config.h | 6 ---- modules/http2/h2_conn_io.c | 11 ++---- modules/http2/h2_ctx.c | 7 ---- modules/http2/h2_ctx.h | 13 ------- modules/http2/h2_h2.c | 69 ++++++++++++++++++++---------------- modules/http2/h2_session.c | 11 +++--- modules/http2/h2_switch.c | 6 ++-- 9 files changed, 53 insertions(+), 207 deletions(-) diff --git a/docs/manual/mod/mod_h2.xml b/docs/manual/mod/mod_h2.xml index 47d8a3a702..c5c77aa373 100644 --- a/docs/manual/mod/mod_h2.xml +++ b/docs/manual/mod/mod_h2.xml @@ -41,7 +41,7 @@ H2Direct H2 Direct Protocol Switch H2Direct on|off - H2Direct on + H2Direct on (for non TLS) server config virtual host @@ -144,29 +144,6 @@ - - H2MaxHeaderListSize - Maximum size of acceptable stream headers. - H2MaxHeaderListSize bytes - H2MaxHeaderListSize 16384 - - server config - virtual host - - -

- This directive sets the maximum amount of stream header bytes that - the server is willing to accept. It is announced to the client during - the initial HTTP/2 handshake. -

- Example - - H2MaxHeaderListSize 10000 - - - - - H2MinWorkers Minimal number of worker threads to use per child process. @@ -233,53 +210,6 @@ - - H2BufferOutput - Output Buffering Switch - H2BufferOutput on|off - - server config - virtual host - - -

- This directive toggles if buffering of HTTP/2 output shall be used - or if data is written immediately when it arrives. Unless specified - otherwise, this directive is on for TLS connections and - off for plain connections. -

- Example - - H2BufferOutput on - - - - - - - H2BufferSize - Buffer size for outgoing data per HTTP/2 connection. - H2BufferSize bytes - H2BufferSize 65536 - - server config - virtual host - - -

- This directive sets the size of the buffer used to hold outgoing - HTTP/2 raw data, should H2BufferOutput be switched on. - This data is allocated per HTTP/2 connection, not stream and is - counted against the raw protocol data. -

- Example - - H2BufferSize 128000 - - - - - H2SessionExtraFiles Number of Extra File Handles diff --git a/modules/http2/h2_config.c b/modules/http2/h2_config.c index 4317978d53..04b0d94a5a 100644 --- a/modules/http2/h2_config.c +++ b/modules/http2/h2_config.c @@ -37,7 +37,6 @@ static h2_config defconf = { "default", 100, /* max_streams */ - 16 * 1024, /* max_hl_size */ 64 * 1024, /* window_size */ -1, /* min workers */ -1, /* max workers */ @@ -46,9 +45,7 @@ static h2_config defconf = { NULL, /* no alt-svcs */ -1, /* alt-svc max age */ 0, /* serialize headers */ - 1, /* h2 direct mode */ - -1, /* buffer output, by default only for TLS */ - 64*1024, /* buffer size */ + -1, /* h2 direct mode */ 5, /* # session extra files */ }; @@ -66,7 +63,6 @@ static void *h2_config_create(apr_pool_t *pool, conf->name = name; conf->h2_max_streams = DEF_VAL; - conf->h2_max_hl_size = DEF_VAL; conf->h2_window_size = DEF_VAL; conf->min_workers = DEF_VAL; conf->max_workers = DEF_VAL; @@ -75,8 +71,6 @@ static void *h2_config_create(apr_pool_t *pool, conf->alt_svc_max_age = DEF_VAL; conf->serialize_headers = DEF_VAL; conf->h2_direct = DEF_VAL; - conf->buffer_output = DEF_VAL; - conf->buffer_size = DEF_VAL; conf->session_extra_files = DEF_VAL; return conf; } @@ -106,7 +100,6 @@ void *h2_config_merge(apr_pool_t *pool, void *basev, void *addv) n->name = name; n->h2_max_streams = H2_CONFIG_GET(add, base, h2_max_streams); - n->h2_max_hl_size = H2_CONFIG_GET(add, base, h2_max_hl_size); n->h2_window_size = H2_CONFIG_GET(add, base, h2_window_size); n->min_workers = H2_CONFIG_GET(add, base, min_workers); n->max_workers = H2_CONFIG_GET(add, base, max_workers); @@ -116,8 +109,6 @@ void *h2_config_merge(apr_pool_t *pool, void *basev, void *addv) n->alt_svc_max_age = H2_CONFIG_GET(add, base, alt_svc_max_age); n->serialize_headers = H2_CONFIG_GET(add, base, serialize_headers); n->h2_direct = H2_CONFIG_GET(add, base, h2_direct); - n->buffer_output = H2_CONFIG_GET(add, base, buffer_output); - n->buffer_size = H2_CONFIG_GET(add, base, buffer_size); n->session_extra_files = H2_CONFIG_GET(add, base, session_extra_files); return n; @@ -128,8 +119,6 @@ int h2_config_geti(h2_config *conf, h2_config_var_t var) switch(var) { case H2_CONF_MAX_STREAMS: return H2_CONFIG_GET(conf, &defconf, h2_max_streams); - case H2_CONF_MAX_HL_SIZE: - return H2_CONFIG_GET(conf, &defconf, h2_max_hl_size); case H2_CONF_WIN_SIZE: return H2_CONFIG_GET(conf, &defconf, h2_window_size); case H2_CONF_MIN_WORKERS: @@ -146,10 +135,6 @@ int h2_config_geti(h2_config *conf, h2_config_var_t var) return H2_CONFIG_GET(conf, &defconf, serialize_headers); case H2_CONF_DIRECT: return H2_CONFIG_GET(conf, &defconf, h2_direct); - case H2_CONF_BUFFER_OUTPUT: - return H2_CONFIG_GET(conf, &defconf, buffer_output); - case H2_CONF_BUFFER_SIZE: - return H2_CONFIG_GET(conf, &defconf, buffer_size); case H2_CONF_SESSION_FILES: return H2_CONFIG_GET(conf, &defconf, session_extra_files); default: @@ -190,18 +175,6 @@ static const char *h2_conf_set_window_size(cmd_parms *parms, return NULL; } -static const char *h2_conf_set_max_hl_size(cmd_parms *parms, - void *arg, const char *value) -{ - h2_config *cfg = h2_config_sget(parms->server); - cfg->h2_max_hl_size = (int)apr_atoi64(value); - (void)arg; - if (cfg->h2_max_hl_size < 1024) { - return "value must be > 1k"; - } - return NULL; -} - static const char *h2_conf_set_min_workers(cmd_parms *parms, void *arg, const char *value) { @@ -279,19 +252,6 @@ static const char *h2_conf_set_alt_svc_max_age(cmd_parms *parms, return NULL; } -static const char *h2_conf_set_buffer_size(cmd_parms *parms, - void *arg, const char *value) -{ - h2_config *cfg = h2_config_sget(parms->server); - apr_int64_t len = (int)apr_atoi64(value); - if (len < (16*1024)) { - return "value must be a positive number, at least 16k"; - } - cfg->buffer_size = (int)len; - (void)arg; - return NULL; -} - static const char *h2_conf_set_session_extra_files(cmd_parms *parms, void *arg, const char *value) { @@ -339,31 +299,12 @@ static const char *h2_conf_set_direct(cmd_parms *parms, return "value must be On or Off"; } -static const char *h2_conf_set_buffer_output(cmd_parms *parms, - void *arg, const char *value) -{ - h2_config *cfg = h2_config_sget(parms->server); - if (!strcasecmp(value, "On")) { - cfg->buffer_output = 1; - return NULL; - } - else if (!strcasecmp(value, "Off")) { - cfg->buffer_output = 0; - return NULL; - } - - (void)arg; - return "value must be On or Off"; -} - #pragma GCC diagnostic ignored "-Wmissing-braces" const command_rec h2_cmds[] = { AP_INIT_TAKE1("H2MaxSessionStreams", h2_conf_set_max_streams, NULL, RSRC_CONF, "maximum number of open streams per session"), AP_INIT_TAKE1("H2WindowSize", h2_conf_set_window_size, NULL, RSRC_CONF, "window size on client DATA"), - AP_INIT_TAKE1("H2MaxHeaderListSize", h2_conf_set_max_hl_size, NULL, - RSRC_CONF, "maximum acceptable size of request headers"), AP_INIT_TAKE1("H2MinWorkers", h2_conf_set_min_workers, NULL, RSRC_CONF, "minimum number of worker threads per child"), AP_INIT_TAKE1("H2MaxWorkers", h2_conf_set_max_workers, NULL, @@ -380,10 +321,6 @@ const command_rec h2_cmds[] = { RSRC_CONF, "on to enable header serialization for compatibility"), AP_INIT_TAKE1("H2Direct", h2_conf_set_direct, NULL, RSRC_CONF, "on to enable direct HTTP/2 mode"), - AP_INIT_TAKE1("H2BufferOutput", h2_conf_set_buffer_output, NULL, - RSRC_CONF, "on to enable output buffering, default for TLS"), - AP_INIT_TAKE1("H2BufferSize", h2_conf_set_buffer_size, NULL, - RSRC_CONF, "size of outgoing buffer in bytes"), AP_INIT_TAKE1("H2SessionExtraFiles", h2_conf_set_session_extra_files, NULL, RSRC_CONF, "number of extra file a session might keep open"), { NULL, NULL, NULL, 0, 0, NULL } diff --git a/modules/http2/h2_config.h b/modules/http2/h2_config.h index 9bd7ea95d6..a13a590514 100644 --- a/modules/http2/h2_config.h +++ b/modules/http2/h2_config.h @@ -24,7 +24,6 @@ typedef enum { H2_CONF_MAX_STREAMS, - H2_CONF_MAX_HL_SIZE, H2_CONF_WIN_SIZE, H2_CONF_MIN_WORKERS, H2_CONF_MAX_WORKERS, @@ -34,8 +33,6 @@ typedef enum { H2_CONF_ALT_SVC_MAX_AGE, H2_CONF_SER_HEADERS, H2_CONF_DIRECT, - H2_CONF_BUFFER_OUTPUT, - H2_CONF_BUFFER_SIZE, H2_CONF_SESSION_FILES, } h2_config_var_t; @@ -43,7 +40,6 @@ typedef enum { typedef struct h2_config { const char *name; int h2_max_streams; /* max concurrent # streams (http2) */ - int h2_max_hl_size; /* max header list size (http2) */ int h2_window_size; /* stream window size (http2) */ int min_workers; /* min # of worker threads/child */ int max_workers; /* max # of worker threads/child */ @@ -54,8 +50,6 @@ typedef struct h2_config { int serialize_headers; /* Use serialized HTTP/1.1 headers for processing, better compatibility */ int h2_direct; /* if mod_h2 is active directly */ - int buffer_output; /* if output buffering shall be used */ - int buffer_size; /* size of buffer for outgoing data */ int session_extra_files; /* # of extra files a session may keep open */ } h2_config; diff --git a/modules/http2/h2_conn_io.c b/modules/http2/h2_conn_io.c index 749965e1fb..129456ebc3 100644 --- a/modules/http2/h2_conn_io.c +++ b/modules/http2/h2_conn_io.c @@ -28,6 +28,7 @@ #include "h2_h2.h" #include "h2_util.h" +#define WRITE_BUFFER_SIZE (64*1024) #define WRITE_SIZE_INITIAL 1300 #define WRITE_SIZE_MAX (16*1024) #define WRITE_SIZE_IDLE_USEC (1*APR_USEC_PER_SEC) @@ -35,8 +36,6 @@ apr_status_t h2_conn_io_init(h2_conn_io *io, conn_rec *c) { - h2_config *cfg = h2_config_get(c); - io->connection = c; io->input = apr_brigade_create(c->pool, c->bucket_alloc); io->output = apr_brigade_create(c->pool, c->bucket_alloc); @@ -45,11 +44,7 @@ apr_status_t h2_conn_io_init(h2_conn_io *io, conn_rec *c) * see https://issues.apache.org/jira/browse/TS-2503 */ io->write_size = WRITE_SIZE_INITIAL; io->last_write = 0; - io->buffer_output = h2_config_geti(cfg, H2_CONF_BUFFER_OUTPUT); - - if (io->buffer_output < 0) { - io->buffer_output = h2_h2_is_tls(c); - } + io->buffer_output = h2_h2_is_tls(c); /* Currently we buffer only for TLS output. The reason this gives * improved performance is that buckets send to the mod_ssl network @@ -59,7 +54,7 @@ apr_status_t h2_conn_io_init(h2_conn_io *io, conn_rec *c) * chunks. */ if (io->buffer_output) { - io->bufsize = h2_config_geti(cfg, H2_CONF_BUFFER_SIZE); + io->bufsize = WRITE_BUFFER_SIZE; io->buffer = apr_pcalloc(c->pool, io->bufsize); } else { diff --git a/modules/http2/h2_ctx.c b/modules/http2/h2_ctx.c index 2894e53e64..0e198456f2 100644 --- a/modules/http2/h2_ctx.c +++ b/modules/http2/h2_ctx.c @@ -28,7 +28,6 @@ static h2_ctx *h2_ctx_create(const conn_rec *c) { h2_ctx *ctx = apr_pcalloc(c->pool, sizeof(h2_ctx)); AP_DEBUG_ASSERT(ctx); - ctx->pnego_state = H2_PNEGO_NONE; ap_set_module_config(c->conn_config, &h2_module, ctx); return ctx; } @@ -65,7 +64,6 @@ const char *h2_ctx_protocol_get(const conn_rec *c) h2_ctx *h2_ctx_protocol_set(h2_ctx *ctx, const char *proto) { ctx->protocol = proto; - ctx->pnego_state = H2_PNEGO_DONE; ctx->is_h2 = (proto != NULL); return ctx; } @@ -75,11 +73,6 @@ int h2_ctx_is_task(h2_ctx *ctx) return ctx && !!ctx->task_env; } -int h2_ctx_pnego_is_ongoing(h2_ctx *ctx) -{ - return ctx && (ctx->pnego_state == H2_PNEGO_STARTED); -} - int h2_ctx_is_active(h2_ctx *ctx) { return ctx && ctx->is_h2; diff --git a/modules/http2/h2_ctx.h b/modules/http2/h2_ctx.h index c47f7ab1e1..f9bc827829 100644 --- a/modules/http2/h2_ctx.h +++ b/modules/http2/h2_ctx.h @@ -19,12 +19,6 @@ struct h2_task_env; struct h2_config; -typedef enum { - H2_PNEGO_NONE, - H2_PNEGO_STARTED, - H2_PNEGO_DONE, -} h2_pnego_state_t; - /** * The h2 module context associated with a connection. * @@ -35,7 +29,6 @@ typedef enum { */ typedef struct h2_ctx { int is_h2; /* h2 engine is used */ - h2_pnego_state_t pnego_state; /* protocol negotiation state */ const char *protocol; /* the protocol negotiated */ struct h2_task_env *task_env; /* the h2_task environment or NULL */ const char *hostname; /* hostname negotiated via SNI, optional */ @@ -53,12 +46,6 @@ h2_ctx *h2_ctx_create_for(const conn_rec *c, struct h2_task_env *env); */ h2_ctx *h2_ctx_protocol_set(h2_ctx *ctx, const char *proto); -/** - * Returns != 0 iff protocol negotiation has started but is not - * done yet. - */ -int h2_ctx_pnego_is_ongoing(h2_ctx *ctx); - /** * Get the h2 protocol negotiated for this connection, or NULL. */ diff --git a/modules/http2/h2_h2.c b/modules/http2/h2_h2.c index 98b6ce77b9..5d729f4621 100644 --- a/modules/http2/h2_h2.c +++ b/modules/http2/h2_h2.c @@ -148,51 +148,59 @@ int h2_h2_process_conn(conn_rec* c) h2_ctx *ctx = h2_ctx_get(c); h2_config *cfg = h2_config_get(c); apr_bucket_brigade* temp; - + int is_tls = h2_h2_is_tls(c); + + ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, "h2_h2, process_conn"); if (h2_ctx_is_task(ctx)) { /* our stream pseudo connection */ return DECLINED; } - /* Protocol negoation, if started, may need some speculative reading - * to get triggered. - */ - if (h2_ctx_pnego_is_ongoing(ctx)) { - temp = apr_brigade_create(c->pool, c->bucket_alloc); - ap_get_brigade(c->input_filters, temp, - AP_MODE_SPECULATIVE, APR_BLOCK_READ, 24); - apr_brigade_destroy(temp); - } - - /* If we have not already switched to a h2* protocol - * and the connection is on "http/1.1" - * and H2Direct is enabled, - * -> sniff for the magic PRIamble. A client sending this on connection - * start should know what it is doing. + /* If we have not already switched to a h2* protocol and the connection + * is on "http/1.1" + * -> sniff for the magic PRIamble. On TLS, this might trigger the ALPN. */ if (!h2_ctx_protocol_get(c) - && !strcmp(AP_PROTOCOL_HTTP1, ap_run_protocol_get(c)) - && h2_config_geti(cfg, H2_CONF_DIRECT)) { + && !strcmp(AP_PROTOCOL_HTTP1, ap_run_protocol_get(c))) { apr_status_t status; + temp = apr_brigade_create(c->pool, c->bucket_alloc); status = ap_get_brigade(c->input_filters, temp, AP_MODE_SPECULATIVE, APR_BLOCK_READ, 24); + if (status == APR_SUCCESS) { - char *s = NULL; - apr_size_t slen; - - apr_brigade_pflatten(temp, &s, &slen, c->pool); - if ((slen >= 24) && !memcmp(H2_MAGIC_TOKEN, s, 24)) { - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, - "h2_h2, direct mode detected"); - h2_ctx_protocol_set(ctx, "h2"); + if (h2_ctx_protocol_get(c) + || strcmp(AP_PROTOCOL_HTTP1, ap_run_protocol_get(c))) { + /* h2 or another protocol has been selected. */ } else { - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, - "h2_h2, not detected, seeing %d bytes: %s", - (int)slen, s); + /* ALPN might have been triggered, but we're still on + * http/1.1. Check the actual bytes read for the H2 Magic + * Token, *if* H2Direct mode is enabled here. + */ + int direct_mode = h2_config_geti(cfg, H2_CONF_DIRECT); + if (direct_mode > 0 || (direct_mode < 0 && !is_tls)) { + char *s = NULL; + apr_size_t slen; + + apr_brigade_pflatten(temp, &s, &slen, c->pool); + if ((slen >= 24) && !memcmp(H2_MAGIC_TOKEN, s, 24)) { + ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, + "h2_h2, direct mode detected"); + h2_ctx_protocol_set(ctx, is_tls? "h2" : "h2c"); + } + else { + ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c, + "h2_h2, not detected in %d bytes: %s", + (int)slen, s); + } + } } } + else { + ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c, + "h2_h2, error reading 24 bytes speculative"); + } apr_brigade_destroy(temp); } @@ -200,12 +208,13 @@ int h2_h2_process_conn(conn_rec* c) * the connection. */ if (h2_ctx_is_active(ctx)) { - ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c, + ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, "h2_h2, connection, h2 active"); return h2_conn_main(c); } + ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, "h2_h2, declined"); return DECLINED; } diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c index 0ce33d095b..710c998943 100644 --- a/modules/http2/h2_session.c +++ b/modules/http2/h2_session.c @@ -725,6 +725,7 @@ apr_status_t h2_session_start(h2_session *session, int *rv) apr_status_t status = APR_SUCCESS; h2_config *config; nghttp2_settings_entry settings[3]; + AP_DEBUG_ASSERT(session); /* Start the conversation by submitting our SETTINGS frame */ *rv = 0; @@ -794,13 +795,13 @@ apr_status_t h2_session_start(h2_session *session, int *rv) } } - settings[0].settings_id = NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE; - settings[0].value = h2_config_geti(config, H2_CONF_MAX_HL_SIZE); + settings[0].settings_id = NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS; + settings[0].value = (uint32_t)session->max_stream_count; settings[1].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE; settings[1].value = h2_config_geti(config, H2_CONF_WIN_SIZE); - settings[2].settings_id = NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS; - settings[2].value = (uint32_t)session->max_stream_count; - + settings[2].settings_id = NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE; + settings[2].value = 64*1024; + *rv = nghttp2_submit_settings(session->ngh2, NGHTTP2_FLAG_NONE, settings, sizeof(settings)/sizeof(settings[0])); diff --git a/modules/http2/h2_switch.c b/modules/http2/h2_switch.c index 2f35bfb704..b0926ce161 100644 --- a/modules/http2/h2_switch.c +++ b/modules/http2/h2_switch.c @@ -135,6 +135,9 @@ static int h2_protocol_switch(conn_rec *c, request_rec *r, server_rec *s, if (found) { h2_ctx *ctx = h2_ctx_get(c); + + ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, + "switching protocol to '%s'", protocol); h2_ctx_protocol_set(ctx, protocol); if (r != NULL) { @@ -154,9 +157,6 @@ static int h2_protocol_switch(conn_rec *c, request_rec *r, server_rec *s, ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, r, "session proessed, unexpected status"); } - } - else { - } return DONE; } -- 2.40.0