Hi, wbr,

Thanks for your review. I don't know why I can't receive your email.
Let me reply directly.

> It doesn't look like a useful feature.
> Could you please explain the use cases?

The current implementation support both http/1 and http/2 over the
same TLS listen port by the ALPN Extension.

Nginx also support listening http/2 over plain tcp port, which is
perfect for development and inner production environment.

However, the current implementation cannot support http/1.1 and http/2
simultaneously. We have no choice but listen on two different ports.
As a result, one same service has two ports and different clients
should choose their suitable port.

Besides, the http/2 is efficient, but http/1 is simple. So I see no
chance that the http/2 will replace http/1 totally in the production
inner environment. Will call the inner API by both http/1 and http/2.

So I think support http/1 and http/2 on the plain tcp port will simplify
both the production and development environment.


> What if the received data is bigger than h2mcf->recv_buffer?

Yes, it is a problem. However, it can be fixed easily.

As we know, the http/2 preface is PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n.
We could modify the ngx_http_parse_request_line and when we got A PRI
method, we need to get a single * uri. If we got things other than *,
we just return a invalid request response. By this, we will never got
a PRI to_much_long_uri HTTP/2.0 request line, and the buffer will not
be exhausted. So the ngx_http_alloc_large_header_buffer will not be called
during the handshake. After the ngx_http_parse_request_line, we will
ensure we got a PRI request, and the buffer size is client_header_buffer_size.

And then we should the compare the r->header_in buffer's length and the
h2mcf->recv_buffer_size. If we got a buffered data larger than the
h2mcf->recv_buffer_size, we should send a bad request response.

If this feature can be accepted, I will send a new patch.

Thanks.


> On Mar 2, 2018, at 15:53, Haitao Lv <i@lvht.net> wrote:
>
> # HG changeset patch
> # User 吕海涛 <i@lvht.net>
> # Date 1519976498 -28800
> # Fri Mar 02 15:41:38 2018 +0800
> # Node ID 200955343460c4726015180f20c03e31c0b35ff6
> # Parent 81fae70d6cb81c67607931ec3ecc585a609c97e0
> make http2 server support http1
>
> diff -r 81fae70d6cb8 -r 200955343460 src/http/ngx_http.c
> --- a/src/http/ngx_http.c Thu Mar 01 20:25:50 2018 +0300
> +++ b/src/http/ngx_http.c Fri Mar 02 15:41:38 2018 +0800
> @@ -1197,6 +1197,7 @@
> ngx_uint_t ssl;
> #endif
> #if (NGX_HTTP_V2)
> + ngx_uint_t http1;
> ngx_uint_t http2;
> #endif
>
> @@ -1232,6 +1233,7 @@
> ssl = lsopt->ssl || addr[i].opt.ssl;
> #endif
> #if (NGX_HTTP_V2)
> + http1 = lsopt->http1 || addr[i].opt.http1;
> http2 = lsopt->http2 || addr[i].opt.http2;
> #endif
>
> @@ -1266,6 +1268,7 @@
> addr[i].opt.ssl = ssl;
> #endif
> #if (NGX_HTTP_V2)
> + addr[i].opt.http1 = http1;
> addr[i].opt.http2 = http2;
> #endif
>
> @@ -1802,6 +1805,7 @@
> addrs[i].conf.ssl = addr[i].opt.ssl;
> #endif
> #if (NGX_HTTP_V2)
> + addrs[i].conf.http1 = addr[i].opt.http1;
> addrs[i].conf.http2 = addr[i].opt.http2;
> #endif
> addrs[i].conf.proxy_protocol = addr[i].opt.proxy_protocol;
> @@ -1867,6 +1871,7 @@
> addrs6[i].conf.ssl = addr[i].opt.ssl;
> #endif
> #if (NGX_HTTP_V2)
> + addrs6[i].conf.http1 = addr[i].opt.http1;
> addrs6[i].conf.http2 = addr[i].opt.http2;
> #endif
> addrs6[i].conf.proxy_protocol = addr[i].opt.proxy_protocol;
> diff -r 81fae70d6cb8 -r 200955343460 src/http/ngx_http_core_module.c
> --- a/src/http/ngx_http_core_module.c Thu Mar 01 20:25:50 2018 +0300
> +++ b/src/http/ngx_http_core_module.c Fri Mar 02 15:41:38 2018 +0800
> @@ -3985,6 +3985,18 @@
> #endif
> }
>
> + if (ngx_strcmp(value[n].data, "http1") == 0) {
> +#if (NGX_HTTP_V2)
> + lsopt.http1 = 1;
> + continue;
> +#else
> + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
> + "the \"http1\" parameter requires "
> + "ngx_http_v2_module");
> + return NGX_CONF_ERROR;
> +#endif
> + }
> +
> if (ngx_strcmp(value[n].data, "spdy") == 0) {
> ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
> "invalid parameter \"spdy\": "
> diff -r 81fae70d6cb8 -r 200955343460 src/http/ngx_http_core_module.h
> --- a/src/http/ngx_http_core_module.h Thu Mar 01 20:25:50 2018 +0300
> +++ b/src/http/ngx_http_core_module.h Fri Mar 02 15:41:38 2018 +0800
> @@ -73,6 +73,7 @@
> unsigned bind:1;
> unsigned wildcard:1;
> unsigned ssl:1;
> + unsigned http1:1;
> unsigned http2:1;
> #if (NGX_HAVE_INET6)
> unsigned ipv6only:1;
> @@ -234,6 +235,7 @@
> ngx_http_virtual_names_t *virtual_names;
>
> unsigned ssl:1;
> + unsigned http1:1;
> unsigned http2:1;
> unsigned proxy_protocol:1;
> };
> diff -r 81fae70d6cb8 -r 200955343460 src/http/ngx_http_parse.c
> --- a/src/http/ngx_http_parse.c Thu Mar 01 20:25:50 2018 +0300
> +++ b/src/http/ngx_http_parse.c Fri Mar 02 15:41:38 2018 +0800
> @@ -285,6 +285,14 @@
> break;
> }
>
> +#if (NGX_HTTP_V2)
> + if (ch == '*') {
> + r->uri_start = p;
> + state = sw_uri;
> + break;
> + }
> +#endif
> +
> c = (u_char) (ch | 0x20);
> if (c >= 'a' && c <= 'z') {
> r->schema_start = p;
> @@ -724,9 +732,11 @@
>
> r->http_major = ch - '0';
>
> +#if !(NGX_HTTP_V2)
> if (r->http_major > 1) {
> return NGX_HTTP_PARSE_INVALID_VERSION;
> }
> +#endif
>
> state = sw_major_digit;
> break;
> @@ -744,9 +754,11 @@
>
> r->http_major = r->http_major * 10 + (ch - '0');
>
> +#if !(NGX_HTTP_V2)
> if (r->http_major > 1) {
> return NGX_HTTP_PARSE_INVALID_VERSION;
> }
> +#endif
>
> break;
>
> diff -r 81fae70d6cb8 -r 200955343460 src/http/ngx_http_request.c
> --- a/src/http/ngx_http_request.c Thu Mar 01 20:25:50 2018 +0300
> +++ b/src/http/ngx_http_request.c Fri Mar 02 15:41:38 2018 +0800
> @@ -17,6 +17,10 @@
> static ngx_int_t ngx_http_alloc_large_header_buffer(ngx_http_request_t *r,
> ngx_uint_t request_line);
>
> +#if (NGX_HTTP_V2)
> +static void ngx_http_process_h2_preface(ngx_event_t *rev);
> +#endif
> +
> static ngx_int_t ngx_http_process_header_line(ngx_http_request_t *r,
> ngx_table_elt_t *h, ngx_uint_t offset);
> static ngx_int_t ngx_http_process_unique_header_line(ngx_http_request_t *r,
> @@ -320,7 +324,7 @@
> c->write->handler = ngx_http_empty_handler;
>
> #if (NGX_HTTP_V2)
> - if (hc->addr_conf->http2) {
> + if (hc->addr_conf->http2 && !hc->addr_conf->http1) {
> rev->handler = ngx_http_v2_init;
> }
> #endif
> @@ -1033,6 +1037,20 @@
> return;
> }
>
> +#if (NGX_HTTP_V2)
> + if (r->http_connection->addr_conf->http2
> + && r->http_version >= NGX_HTTP_VERSION_20) {
> +
> + c->log->action = "reading client h2 preface";
> +
> + rev->handler = ngx_http_process_h2_preface;
> + ngx_http_process_h2_preface(rev);
> + return;
> + } else if (r->http_version >= NGX_HTTP_VERSION_20) {
> + ngx_http_finalize_request(r, NGX_HTTP_VERSION_NOT_SUPPORTED);
> + return;
> + }
> +#endif
>
> if (ngx_list_init(&r->headers_in.headers, r->pool, 20,
> sizeof(ngx_table_elt_t))
> @@ -1208,6 +1226,64 @@
> return NGX_OK;
> }
>
> +#if (NGX_HTTP_V2)
> +static void
> +ngx_http_process_h2_preface(ngx_event_t *rev)
> +{
> + size_t len;
> + ssize_t n;
> + ngx_connection_t *c;
> + ngx_http_request_t *r;
> +
> + c = rev->data;
> + r = c->data;
> +
> + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0,
> + "http process h2 preface");
> +
> + if (rev->timedout) {
> + ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "client timed out");
> + c->timedout = 1;
> + ngx_http_close_request(r, NGX_HTTP_REQUEST_TIME_OUT);
> + return;
> + }
> +
> + n = ngx_http_read_request_header(r);
> +
> + if (n == NGX_AGAIN || n == NGX_ERROR) {
> + return;
> + }
> +
> + len = r->header_in->last - r->header_in->start;
> +
> + if (len < sizeof("PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n") - 1) {
> + r->header_in->pos = r->header_in->last;
> + return;
> + }
> +
> + if (ngx_strncmp(r->header_in->start,
> + "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n", 24) == 0) {
> +
> + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0,
> + "h2 preface done");
> +
> + r->header_in->pos = r->header_in->start + 24;
> +
> + c->data = r->http_connection;
> +
> + r->http_connection = NULL;
> +
> + ngx_http_v2_init_after_preface(rev, r->header_in);
> + return;
> + }
> +
> + ngx_log_error(NGX_LOG_INFO, c->log, 0,
> + "client sent invalid h2 preface");
> +
> + ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST);
> +}
> +#endif
> +
>
> static void
> ngx_http_process_request_headers(ngx_event_t *rev)
> @@ -1809,7 +1885,7 @@
> return NGX_ERROR;
> }
>
> - if (r->headers_in.host == NULL && r->http_version > NGX_HTTP_VERSION_10) {
> + if (r->headers_in.host == NULL && r->http_version == NGX_HTTP_VERSION_11) {
> ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
> "client sent HTTP/1.1 request without \"Host\" header");
> ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST);
> diff -r 81fae70d6cb8 -r 200955343460 src/http/v2/ngx_http_v2.c
> --- a/src/http/v2/ngx_http_v2.c Thu Mar 01 20:25:50 2018 +0300
> +++ b/src/http/v2/ngx_http_v2.c Fri Mar 02 15:41:38 2018 +0800
> @@ -231,6 +231,13 @@
> void
> ngx_http_v2_init(ngx_event_t *rev)
> {
> + ngx_http_v2_init_after_preface(rev, NULL);
> +}
> +
> +
> +void
> +ngx_http_v2_init_after_preface(ngx_event_t *rev, ngx_buf_t *buf)
> +{
> ngx_connection_t *c;
> ngx_pool_cleanup_t *cln;
> ngx_http_connection_t *hc;
> @@ -316,6 +323,12 @@
> h2c->state.handler = hc->proxy_protocol ? ngx_http_v2_state_proxy_protocol
> : ngx_http_v2_state_preface;
>
> + if (buf != NULL) {
> + ngx_memcpy(h2mcf->recv_buffer, buf->pos, buf->last - buf->pos);
> + h2c->state.buffer_used = buf->last - buf->pos;
> + h2c->state.handler = ngx_http_v2_state_head;
> + }
> +
> ngx_queue_init(&h2c->waiting);
> ngx_queue_init(&h2c->dependencies);
> ngx_queue_init(&h2c->closed);
> @@ -381,13 +394,18 @@
> h2mcf = ngx_http_get_module_main_conf(h2c->http_connection->conf_ctx,
> ngx_http_v2_module);
>
> - available = h2mcf->recv_buffer_size - 2 * NGX_HTTP_V2_STATE_BUFFER_SIZE;
> + available = h2mcf->recv_buffer_size - h2c->state.buffer_used - 2 * NGX_HTTP_V2_STATE_BUFFER_SIZE;
>
> do {
> p = h2mcf->recv_buffer;
>
> + end = p + h2c->state.buffer_used;
> +
> + if (h2c->state.buffer_used) {
> + goto do_state_handler;
> + }
> +
> ngx_memcpy(p, h2c->state.buffer, NGX_HTTP_V2_STATE_BUFFER_SIZE);
> - end = p + h2c->state.buffer_used;
>
> n = c->recv(c, end, available);
>
> @@ -410,6 +428,8 @@
>
> end += n;
>
> +do_state_handler:
> +
> h2c->state.buffer_used = 0;
> h2c->state.incomplete = 0;
>
> diff -r 81fae70d6cb8 -r 200955343460 src/http/v2/ngx_http_v2.h
> --- a/src/http/v2/ngx_http_v2.h Thu Mar 01 20:25:50 2018 +0300
> +++ b/src/http/v2/ngx_http_v2.h Fri Mar 02 15:41:38 2018 +0800
> @@ -279,6 +279,7 @@
>
>
> void ngx_http_v2_init(ngx_event_t *rev);
> +void ngx_http_v2_init_after_preface(ngx_event_t *rev, ngx_buf_t *buf);
>
> ngx_int_t ngx_http_v2_read_request_body(ngx_http_request_t *r);
> ngx_int_t ngx_http_v2_read_unbuffered_request_body(ngx_http_request_t *r);
>



_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel