Re: [PATCH] HTTP/2: make http2 server support http1

[Haitao Lv]

   if (b != NULL) {
+size = b->last - b->start;
+
+if (size > h2mcf->recv_buffer_size) {
+size = h2mcf->recv_buffer_size;
+}
+
+ngx_memcpy(h2mcf->recv_buffer, b->start, size);
+h2c->state.buffer_used = size;
+
+ngx_pfree(c->pool, b->start);
+ngx_pfree(c->pool, b);
+c->buffer = NULL;
+}
+
 h2c->connection = c;
 h2c->http_connection = hc;
 
@@ -381,13 +400,15 @@ ngx_http_v2_read_handler(ngx_event_t *rev)
 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;
 
-ngx_memcpy(p, h2c->state.buffer, NGX_HTTP_V2_STATE_BUFFER_SIZE);
 end = p + h2c->state.buffer_used;
+if (h2c->state.buffer_used == 0) {
+ngx_memcpy(p, h2c->state.buffer, NGX_HTTP_V2_STATE_BUFFER_SIZE);
+}
 
 n = c->recv(c, end, available);
 


> On Mar 6, 2018, at 10:19, Haitao Lv <i...@lvht.net> wrote:
> 
> Hello, here is another patch(more sample) according Maxim Dounin advice.
> 
> Please offer your comment. Thanks.
> 
> diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
> index 89cfe77a..71bc7b59 100644
> --- a/src/http/ngx_http_request.c
> +++ b/src/http/ngx_http_request.c
> @@ -17,6 +17,10 @@ static ssize_t 
> ngx_http_read_request_header(ngx_http_request_t *r);
> 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_wait_v2_preface_handler(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,
> @@ -321,7 +325,7 @@ ngx_http_init_connection(ngx_connection_t *c)
> 
> #if (NGX_HTTP_V2)
> if (hc->addr_conf->http2) {
> -rev->handler = ngx_http_v2_init;
> +rev->handler = ngx_http_wait_v2_preface_handler;
> }
> #endif
> 
> @@ -377,6 +381,108 @@ ngx_http_init_connection(ngx_connection_t *c)
> }
> 
> 
> +#if (NGX_HTTP_V2)
> +static void
> +ngx_http_wait_v2_preface_handler(ngx_event_t *rev)
> +{
> +size_t size;
> +ssize_tn;
> +ngx_buf_t *b;
> +ngx_connection_t  *c;
> +
> +c = rev->data;
> +
> +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0,
> +"http wait h2 preface handler");
> +
> +if (rev->timedout) {
> +ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "client timed 
> out");
> +ngx_http_close_connection(c);
> +return;
> +}
> +
> +if (c->close) {
> +ngx_http_close_connection(c);
> +return;
> +}
> +
> +size = 5 /* strlen("PRI *") */;
> +
> +b = c->buffer;
> +
> +if (b == NULL) {
> +b = ngx_create_temp_buf(c->pool, size);
> +if (b == NULL) {
> +ngx_http_close_connection(c);
> +return;
> +}
> +
> +c->buffer = b;
> +
> +} else if (b->start == NULL) {
> +
> +b->start = ngx_palloc(c->pool, size);
> +if (b->start == NULL) {
> +ngx_http_close_connection(c);
> +return;
> +}
> +
> +b->pos = b->start;
> +b->last = b->start;
> +b->end = b->last + size;
> +}
> +
> +n = c->recv(c, b->last, size);
> +
> +if (n == NGX_AGAIN) {
> +
> +if (!rev->timer_set) {
> +ngx_add_timer(rev, c->listening->post_accept_timeout);
> +ngx_reusable_connection(c, 1);
> +}
> +
> +if (ngx_handle_read_event(rev, 0) != NGX_OK) {
> +ngx_http_close_connection(c);
> +return;
> +}
> +
> +/*
> + * We are trying to not hold c->buffer's memory for an idle 
> connection.
> + */
> +
> +if (ngx_pfree(c->pool, b->start) == NGX_OK) {
> +b->start = NULL;
> +}
> +
> +return;
> +}
> +
> +if (n == NGX_ERROR) {
> +ngx_http_close_connection(c);
> +return;
> +}
> +
> +if (n == 0) {
> +ngx_log_error(NGX_LOG_INFO, c->log

Re: [PATCH] HTTP/2: make http2 server support http1

[Haitao Lv]

nit(ngx_event_t *rev)
 return;
 }
 
+b = c->buffer;
+
+if (b != NULL) {
+size = b->last - b->start;
+
+if (size > h2mcf->recv_buffer_size) {
+size = h2mcf->recv_buffer_size;
+}
+
+ngx_memcpy(h2mcf->recv_buffer, b->start, size);
+h2c->state.buffer_used = size;
+
+ngx_pfree(c->pool, b->start);
+ngx_pfree(c->pool, b);
+c->buffer = NULL;
+}
+
 h2c->connection = c;
 h2c->http_connection = hc;
 
@@ -381,13 +400,15 @@ ngx_http_v2_read_handler(ngx_event_t *rev)
 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;
 
-ngx_memcpy(p, h2c->state.buffer, NGX_HTTP_V2_STATE_BUFFER_SIZE);
 end = p + h2c->state.buffer_used;
+if (h2c->state.buffer_used == 0) {
+ngx_memcpy(p, h2c->state.buffer, NGX_HTTP_V2_STATE_BUFFER_SIZE);
+}
 
 n = c->recv(c, end, available);
 



> On Mar 6, 2018, at 03:14, Maxim Dounin <mdou...@mdounin.ru> wrote:
> 
> Hello!
> 
> On Mon, Mar 05, 2018 at 11:52:57PM +0800, Haitao Lv wrote:
> 
> [...]
> 
>>> Overall, the patch looks like a hack and introduces too much
>>> complexity for this feature.  While I understand the reasoning,
>>> the proposed implementation cannot be accepted.
>> 
>> Could you clarify that whether is this feature not accepted or this patch?
>> 
>> If this feature is not needed, I will terminate this thread.
>> 
>> If this patch only looks like a hack, would you like offer any advice to 
>> write
>> code with good smell?
> 
> We've previously discussed this with Valentin, and our position is 
> as follows:
> 
> - The feature itself (autodetection between HTTP/2 and HTTP/1.x 
>  protocols) might be usable, and we can consider adding it if 
>  there will be a good and simple enough patch.  (Moreover, we 
>  think that this probably should be the default if "listen ... 
>  http2" is configured - that is, no "http1" option.)
> 
> - The patch suggested certainly doesn't meet the above criteria, 
>  and it does not look like it can be fixed.
> 
> We don't know if a good and simple enough implementation is at all 
> possible though.  One of the possible approaches was already 
> proposed by Valentin (detect HTTP/2 or HTTP/1.x before starting 
> processing, may be similar to how we handle http-to-https 
> requests), but it's now immediately clear if it will work or not.  
> Sorry, but please don't expect any of us to provide further 
> guidance.
> 
> -- 
> Maxim Dounin
> http://mdounin.ru/
> ___
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel



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

Re: [PATCH] HTTP/2: make http2 server support http1

[Haitao Lv]

Any body is here?

> On Mar 21, 2018, at 11:36, Haitao Lv <i...@lvht.net> wrote:
> 
> Thank you for reviewing.
> 
> And here is the patch that fix the breaking PROXY protocol functionality.
> 
> Sorry for disturbing.
> 
> diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
> index 2db7a627..9f1b8544 100644
> --- a/src/http/ngx_http_request.c
> +++ b/src/http/ngx_http_request.c
> @@ -17,6 +17,10 @@ static ssize_t 
> ngx_http_read_request_header(ngx_http_request_t *r);
> 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_wait_v2_preface_handler(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,
> @@ -325,7 +329,7 @@ ngx_http_init_connection(ngx_connection_t *c)
> 
> #if (NGX_HTTP_V2)
> if (hc->addr_conf->http2) {
> -rev->handler = ngx_http_v2_init;
> +rev->handler = ngx_http_wait_v2_preface_handler;
> }
> #endif
> 
> @@ -381,6 +385,131 @@ ngx_http_init_connection(ngx_connection_t *c)
> }
> 
> 
> +#if (NGX_HTTP_V2)
> +static void
> +ngx_http_wait_v2_preface_handler(ngx_event_t *rev)
> +{
> +size_t size;
> +ssize_tn;
> +u_char*p;
> +ngx_buf_t *b;
> +ngx_connection_t  *c;
> +ngx_http_connection_t *hc;
> +static const u_charpreface[] = "PRI";
> +
> +c = rev->data;
> +hc = c->data;
> +
> +size = sizeof(preface) - 1;
> +
> +if (hc->proxy_protocol) {
> +size += NGX_PROXY_PROTOCOL_MAX_HEADER;
> +}
> +
> +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0,
> +"http wait h2 preface handler");
> +
> +if (rev->timedout) {
> +ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "client timed 
> out");
> +ngx_http_close_connection(c);
> +return;
> +}
> +
> +if (c->close) {
> +ngx_http_close_connection(c);
> +return;
> +}
> +
> +b = c->buffer;
> +
> +if (b == NULL) {
> +b = ngx_create_temp_buf(c->pool, size);
> +if (b == NULL) {
> +ngx_http_close_connection(c);
> +return;
> +}
> +
> +c->buffer = b;
> +
> +} else if (b->start == NULL) {
> +
> +b->start = ngx_palloc(c->pool, size);
> +if (b->start == NULL) {
> +ngx_http_close_connection(c);
> +return;
> +}
> +
> +b->pos = b->start;
> +b->last = b->start;
> +b->end = b->last + size;
> +}
> +
> +n = c->recv(c, b->last, b->end - b->last);
> +
> +if (n == NGX_AGAIN) {
> +
> +if (!rev->timer_set) {
> +ngx_add_timer(rev, c->listening->post_accept_timeout);
> +ngx_reusable_connection(c, 1);
> +}
> +
> +if (ngx_handle_read_event(rev, 0) != NGX_OK) {
> +ngx_http_close_connection(c);
> +return;
> +}
> +
> +/*
> + * We are trying to not hold c->buffer's memory for an idle 
> connection.
> + */
> +
> +if (ngx_pfree(c->pool, b->start) == NGX_OK) {
> +b->start = NULL;
> +}
> +
> +return;
> +}
> +
> +if (n == NGX_ERROR) {
> +ngx_http_close_connection(c);
> +return;
> +}
> +
> +if (n == 0) {
> +ngx_log_error(NGX_LOG_INFO, c->log, 0,
> +  "client closed connection");
> +ngx_http_close_connection(c);
> +return;
> +}
> +
> +b->last += n;
> +
> +if (hc->proxy_protocol) {
> +hc->proxy_protocol = 0;
> +
> +p = ngx_proxy_protocol_read(c, b->pos, b->last);
> +
> +if (p == NULL) {
> +ngx_http_close_connection(c);
> +return;
> +}
> +
> +b->pos = p;
> +}
> +
> +if (b->last >= b->pos + sizeof(preface) - 1) {
> +/* b will be freed in ngx_http_v2_init/ngx_http_wait_request_handler 
> */
> +
> +if (ngx_strncmp(b->pos, preface, sizeof(preface) - 1) == 0) {
> +ngx_http_v2_init(rev);
> +} else {
> +rev->handler = ngx_http_wait_request_h

Re: [PATCH] HTTP/2: make http2 server support http1

[Haitao Lv]

ping @wbr

> On Mar 13, 2018, at 01:11, Valentin V. Bartenev <vb...@nginx.com> wrote:
> 
> I will look when time permits.
> 
> But at the first glance the patch still look too complicated than it should 
> be.
> 
>  wbr, Valentin V. Bartenev
> 
> 
> On Tuesday 13 March 2018 00:44:28 吕海涛 wrote:
>> Is there any one who would like to review this patch?
>> 
>> 发自我的 iPhone
>> 
>>> 在 2018年3月8日，08:42，Haitao Lv <i...@lvht.net> 写道：
>>> 
>>> Sorry for disturbing. But I have to fix a buffer overflow bug.
>>> Here is the latest patch.
>>> 
>>> Sorry. But please make your comments. Thank you.
>>> 
>>> 
>>> diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
>>> index 89cfe77a..c51d8ace 100644
>>> --- a/src/http/ngx_http_request.c
>>> +++ b/src/http/ngx_http_request.c
>>> @@ -17,6 +17,10 @@ static ssize_t 
>>> ngx_http_read_request_header(ngx_http_request_t *r);
>>> 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_wait_v2_preface_handler(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,
>>> @@ -321,7 +325,7 @@ ngx_http_init_connection(ngx_connection_t *c)
>>> 
>>> #if (NGX_HTTP_V2)
>>>   if (hc->addr_conf->http2) {
>>> -rev->handler = ngx_http_v2_init;
>>> +rev->handler = ngx_http_wait_v2_preface_handler;
>>>   }
>>> #endif
>>> 
>>> @@ -377,6 +381,110 @@ ngx_http_init_connection(ngx_connection_t *c)
>>> }
>>> 
>>> 
>>> +#if (NGX_HTTP_V2)
>>> +static void
>>> +ngx_http_wait_v2_preface_handler(ngx_event_t *rev)
>>> +{
>>> +size_t size;
>>> +ssize_tn;
>>> +ngx_buf_t *b;
>>> +ngx_connection_t  *c;
>>> +static const u_charpreface[] = "PRI";
>>> +
>>> +c = rev->data;
>>> +size = sizeof(preface) - 1;
>>> +
>>> +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0,
>>> +"http wait h2 preface handler");
>>> +
>>> +if (rev->timedout) {
>>> +ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "client timed 
>>> out");
>>> +ngx_http_close_connection(c);
>>> +return;
>>> +}
>>> +
>>> +if (c->close) {
>>> +ngx_http_close_connection(c);
>>> +return;
>>> +}
>>> +
>>> +b = c->buffer;
>>> +
>>> +if (b == NULL) {
>>> +b = ngx_create_temp_buf(c->pool, size);
>>> +if (b == NULL) {
>>> +ngx_http_close_connection(c);
>>> +return;
>>> +}
>>> +
>>> +c->buffer = b;
>>> +
>>> +} else if (b->start == NULL) {
>>> +
>>> +b->start = ngx_palloc(c->pool, size);
>>> +if (b->start == NULL) {
>>> +ngx_http_close_connection(c);
>>> +return;
>>> +}
>>> +
>>> +b->pos = b->start;
>>> +b->last = b->start;
>>> +b->end = b->last + size;
>>> +}
>>> +
>>> +n = c->recv(c, b->last, b->end - b->last);
>>> +
>>> +if (n == NGX_AGAIN) {
>>> +
>>> +if (!rev->timer_set) {
>>> +ngx_add_timer(rev, c->listening->post_accept_timeout);
>>> +ngx_reusable_connection(c, 1);
>>> +}
>>> +
>>> +if (ngx_handle_read_event(rev, 0) != NGX_OK) {
>>> +ngx_http_close_connection(c);
>>> +return;
>>> +}
>>> +
>>> +/*
>>> + * We are trying to not hold c->buffer's memory for an idle 
>>> connection.
>>> + */
>>> +
>>> +if (ngx_pfree(c->pool, b->start) == NGX_OK) {
>>> +b->start = NULL;
>>> +}
>>> +
>>> +   

Re: [PATCH] HTTP/2: make http2 server support http1

[Haitao Lv]

t);
+
+b->start = p;
+b->pos = b->start;
+b->last = b->start + n;
+b->end = b->last + size;
 }
 
-n = c->recv(c, b->last, size);
+if (n == NGX_AGAIN) {
+n = c->recv(c, b->last, size);
+}
 
 if (n == NGX_AGAIN) {
 
diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c
index 77ebb847..6724b662 100644
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -229,6 +229,8 @@ static ngx_http_v2_parse_header_t  
ngx_http_v2_parse_headers[] = {
 void
 ngx_http_v2_init(ngx_event_t *rev)
 {
+size_t size;
+ngx_buf_t *b;
 ngx_connection_t  *c;
 ngx_pool_cleanup_t*cln;
 ngx_http_connection_t *hc;
@@ -260,6 +262,23 @@ ngx_http_v2_init(ngx_event_t *rev)
 return;
 }
 
+b = c->buffer;
+
+if (b != NULL) {
+size = b->last - b->pos;
+
+if (size > h2mcf->recv_buffer_size) {
+size = h2mcf->recv_buffer_size;
+}
+
+ngx_memcpy(h2mcf->recv_buffer, b->pos, size);
+h2c->state.buffer_used = size;
+
+ngx_pfree(c->pool, b->start);
+ngx_pfree(c->pool, b);
+c->buffer = NULL;
+}
+
 h2c->connection = c;
 h2c->http_connection = hc;
 
@@ -379,13 +398,15 @@ ngx_http_v2_read_handler(ngx_event_t *rev)
 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;
 
-ngx_memcpy(p, h2c->state.buffer, NGX_HTTP_V2_STATE_BUFFER_SIZE);
     end = p + h2c->state.buffer_used;
+if (h2c->state.buffer_used == 0) {
+ngx_memcpy(p, h2c->state.buffer, NGX_HTTP_V2_STATE_BUFFER_SIZE);
+}
 
 n = c->recv(c, end, available);
 


> On Mar 21, 2018, at 00:02, Valentin V. Bartenev <vb...@nginx.com> wrote:
> 
> On Thursday 08 March 2018 08:42:27 Haitao Lv wrote:
>> Sorry for disturbing. But I have to fix a buffer overflow bug.
>> Here is the latest patch.
>> 
>> Sorry. But please make your comments. Thank you.
> [..]
> 
> There's no way for this patch to be accepted as it breaks PROXY protocol
> functionality.
> 
>  wbr, Valentin V. Bartenev
> 
> ___
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel



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

Re: [PATCH] HTTP/2: make http2 server support http1

[Haitao Lv]

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 Fr

Re: [PATCH] HTTP/2: make http2 server support http1

[Haitao Lv]

gt;pos;
+
+h2mcf = ngx_http_get_module_main_conf(hc->conf_ctx, ngx_http_v2_module);
+
+if (len <= h2mcf->recv_buffer_size) {
+c->data = r->http_connection;
+
+ngx_http_free_request(r, -1);
+
+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 +1869,7 @@ ngx_http_process_request_header(ngx_http_request_t *r)
 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);
@@ -3506,7 +3566,9 @@ ngx_http_free_request(ngx_http_request_t *r, ngx_int_t rc)
 
 log->action = "logging request";
 
-ngx_http_log_request(r);
+if (rc >= 0) {
+ngx_http_log_request(r);
+}
 
 log->action = "closing request";
 
diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h
index 5d44c06e..34140655 100644
--- a/src/http/ngx_http_request.h
+++ b/src/http/ngx_http_request.h
@@ -41,6 +41,7 @@
 #define NGX_HTTP_UNLOCK0x2000
 #define NGX_HTTP_PATCH 0x4000
 #define NGX_HTTP_TRACE 0x8000
+#define NGX_HTTP_PRI 0x01
 
 #define NGX_HTTP_CONNECTION_CLOSE  1
 #define NGX_HTTP_CONNECTION_KEEP_ALIVE 2
diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c
index d9df0f90..ea72f129 100644
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -230,6 +230,13 @@ static ngx_http_v2_parse_header_t  
ngx_http_v2_parse_headers[] = {
 
 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;
@@ -316,6 +323,12 @@ ngx_http_v2_init(ngx_event_t *rev)
 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(>waiting);
 ngx_queue_init(>dependencies);
 ngx_queue_init(>closed);
@@ -381,13 +394,17 @@ ngx_http_v2_read_handler(ngx_event_t *rev)
 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;
 
-ngx_memcpy(p, h2c->state.buffer, NGX_HTTP_V2_STATE_BUFFER_SIZE);
 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);
 
 n = c->recv(c, end, available);
 
@@ -410,6 +427,8 @@ ngx_http_v2_read_handler(ngx_event_t *rev)
 
 end += n;
 
+do_state_handler:
+
 h2c->state.buffer_used = 0;
 h2c->state.incomplete = 0;
 
diff --git a/src/http/v2/ngx_http_v2.h b/src/http/v2/ngx_http_v2.h
index d89e8fef..cc9251f0 100644
--- a/src/http/v2/ngx_http_v2.h
+++ b/src/http/v2/ngx_http_v2.h
@@ -279,6 +279,7 @@ ngx_http_v2_queue_ordered_frame(ngx_http_v2_connection_t 
*h2c,
 
 
 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);


> On Mar 4, 2018, at 10:53, Haitao Lv <i...@lvht.net> wrote:
> 
> 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.
> 
> Howev

Re: [PATCH] HTTP/2: make http2 server support http1

[Haitao Lv]

> On Mar 4, 2018, at 21:00, Valentin V. Bartenev <vb...@nginx.com> wrote:
> 
> On Sunday, 4 March 2018 05:53:36 MSK Haitao Lv wrote:
>> 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.
>> 
> 
> HTTP/2 was designed to save connection time between a client and a web
> server, which can be costly with significant RTT and full TLS handshake.
> 
> Also it helps to overcome concurrency limitation in browsers, that usually
> limited to 6 connection per host.
> 
> All these problems usually don't exist in inner environments.
> 
> HTTP/2 isn't efficient.  In fact, it has more overhead and may require
> more TCP packets to transfer the same amount of data.  Moreover, more TCP
> connections are usually faster than one, just because they have bigger start
> window in total and less suffer from packet loss.
> 
> Please, don't be fooled by aggressive marketing around HTTP/2.  Most of
> such articles are written by people who barely understand how network works.
> HTTP/2 is neither a better version nor the "next" version of HTTP protocol
> (the name is misleading).  It's an another protocol designed to solve some
> specific cases, while introducing many other problems.
> 
> I don't recommend to use HTTP/2 in cases other than TLS connections between
> browsers and web servers in public network.  Also even for this purpose,
> there are cases when HTTP/2 isn't recommended, e.g. unreliable networks
> with packet loss (as mobile networks).
> 
> You can easily find on YouTube a talk by Hooman Beheshti called
> "HTTP/2: what no one is telling you" with some interesting research.
> 
> nginx support HTTP/2 over plain TCP for a number of reasons:
> 
> 1. for easy testing and debugging the protocol implementation in nginx;
> 
> 2. that required almost no additional code to implement;
> 
> 3. there's a use case when TLS termination is done by simple TCP proxy
>and then the traffic routed to HTTP/1 or HTTP/2 nginx ports according
>to ALPN.

My actual use case is simple. My compony is going to introduce the gRPC.
And the gRPC depends on the HTTP/2 and it's trailer headers. We have many php
legacy code which expose API by HTTP/1. So we want to build gRPC server by PHP.

A simple solution is to send gRPC payload by HTTP/1 and let nginx transfer it
to HTTP/2. And the grpc-status, grpc-message header that should be transferred
by the HTTP/2 trailer header could be transferred by the normal HTTP/1 header.

By this way, we can support both HTTP/1 API and unary-call gRPC.

> 
>> 
>>> 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.
>> 
> 
> As far as I can see, your reasoning is based on assumption that
> client_header_buffer_size is always smaller than http2_recv_buffer_size.
> 
> That simply isn't true as both can be easily configured by users.

Let me explain in the new patch.
> 
>  wbr, Valentin V. Bartenev
> 
> ___
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel



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

Re: [PATCH] HTTP/2: make http2 server support http1

[Haitao Lv]

@wbr

> On Mar 4, 2018, at 13:49, Haitao Lv <i...@lvht.net> wrote:
> 
> Here is the new patch
> 
> diff --git a/src/http/ngx_http.c b/src/http/ngx_http.c
> index 9d8b6d79..c06ef7c7 100644
> --- a/src/http/ngx_http.c
> +++ b/src/http/ngx_http.c
> @@ -1197,6 +1197,7 @@ ngx_http_add_addresses(ngx_conf_t *cf, 
> ngx_http_core_srv_conf_t *cscf,
> ngx_uint_t ssl;
> #endif
> #if (NGX_HTTP_V2)
> +ngx_uint_t http1;
> ngx_uint_t http2;
> #endif
> 
> @@ -1232,6 +1233,7 @@ ngx_http_add_addresses(ngx_conf_t *cf, 
> ngx_http_core_srv_conf_t *cscf,
> 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 @@ ngx_http_add_addresses(ngx_conf_t *cf, 
> ngx_http_core_srv_conf_t *cscf,
> 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 @@ ngx_http_add_addrs(ngx_conf_t *cf, ngx_http_port_t 
> *hport,
> 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 @@ ngx_http_add_addrs6(ngx_conf_t *cf, ngx_http_port_t 
> *hport,
> 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 --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
> index 6b318dd0..741a4dd9 100644
> --- a/src/http/ngx_http_core_module.c
> +++ b/src/http/ngx_http_core_module.c
> @@ -3985,6 +3985,18 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx_command_t 
> *cmd, void *conf)
> #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 --git a/src/http/ngx_http_core_module.h b/src/http/ngx_http_core_module.h
> index d7985049..68b3e7d9 100644
> --- a/src/http/ngx_http_core_module.h
> +++ b/src/http/ngx_http_core_module.h
> @@ -73,6 +73,7 @@ typedef struct {
> 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 @@ struct ngx_http_addr_conf_s {
> ngx_http_virtual_names_t  *virtual_names;
> 
> unsigned   ssl:1;
> +unsigned   http1:1;
> unsigned   http2:1;
> unsigned   proxy_protocol:1;
> };
> diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c
> index 844054c9..9eb713b7 100644
> --- a/src/http/ngx_http_parse.c
> +++ b/src/http/ngx_http_parse.c
> @@ -117,6 +117,7 @@ ngx_http_parse_request_line(ngx_http_request_t *r, 
> ngx_buf_t *b)
> sw_host_ip_literal,
> sw_port,
> sw_host_http_09,
> +sw_h2_preface,
> sw_after_slash_in_uri,
> sw_check_uri,
> sw_check_uri_http_09,
> @@ -134,6 +135,12 @@ ngx_http_parse_request_line(ngx_http_request_t *r, 
> ngx_buf_t *b)
> sw_almost_done
> } state;
> 
> +#if (NGX_HTTP_V2)
> +static u_char h2_preface[] = "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n";
> +u_char *h2_preface_end = h2_preface + 24;
> +u_char *n = h2_preface + 4;
> +#endif
> +
> state = r->state;
> 
> for (p = b->pos; p < b->last; p++) {
> @@ -145,10 +152,6 @@ ngx_http_parse_request_line(ngx_http_request_t *r, 
> ngx_buf_t *b)
> case sw_start:
> r->request_start = p;
> 
> -if (ch == CR || ch == LF) {
> -break;
> - 

Re: [PATCH] HTTP/2: make http2 server support http1

[Haitao Lv]

> On Mar 5, 2018, at 17:02, Ruslan Ermilov <r...@nginx.com> wrote:
> 
> On Mon, Mar 05, 2018 at 11:11:08AM +0800, Haitao Lv wrote:
> [...]
>>> @@ -145,10 +152,6 @@ ngx_http_parse_request_line(ngx_http_request_t *r, 
>>> ngx_buf_t *b)
>>>case sw_start:
>>>r->request_start = p;
>>> 
>>> -if (ch == CR || ch == LF) {
>>> -break;
>>> -}
>>> -
>> 
>> I think Nginx should not allow any leading \r or \n. HTTP client should 
>> never send this chars
>> before the request line. Support this feature makes the buffer management 
>> more harder.
> 
> https://tools.ietf.org/html/rfc2616#section-4.1
> 
> In the interest of robustness, servers SHOULD ignore any empty
> line(s) received where a Request-Line is expected. In other words, if
> the server is reading the protocol stream at the beginning of a
> message and receives a CRLF first, it should ignore the CRLF.
> 
> 
> https://tools.ietf.org/html/rfc7230#section-3.5
> 
> In the interest of robustness, a server that is expecting to receive
> and parse a request-line SHOULD ignore at least one empty line (CRLF)
> received prior to the request-line.

Thank you, it is a mistake.

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



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

Re: [PATCH] HTTP/2: make http2 server support http1

[Haitao Lv]

Hello wbr,

Thank you for reviewing this patch.

> On Mar 5, 2018, at 23:06, Valentin V. Bartenev <vb...@nginx.com> wrote:
> 
> On Monday 05 March 2018 11:11:08 Haitao Lv wrote:
> [..]
>>> @@ -145,10 +152,6 @@ ngx_http_parse_request_line(ngx_http_request_t *r, 
>>> ngx_buf_t *b)
>>>case sw_start:
>>>r->request_start = p;
>>> 
>>> -if (ch == CR || ch == LF) {
>>> -break;
>>> -}
>>> -
>> 
>> I think Nginx should not allow any leading \r or \n. HTTP client should 
>> never send this chars
>> before the request line. Support this feature makes the buffer management 
>> more harder.
> 
> Support for this feature is required.  See the notes from Ruslan.
> Some clients send additional empty line between keepalive requests.

Yes, this is a mistake. I did not read the RFC carefully. Sorry again. But this 
change isn't needed.

> 
> [..]
>>> +#if (NGX_HTTP_V2)
>>> +case sw_h2_preface:
>>> +
>>> +if (ch == *n++) {
>> 
>> I introduce a new sw_h2_preface state. In this state, the parser will check 
>> the h2 preface char by char.
>> 
>> The parser only check at most 20 chars "* HTTP/2.0\r\n\r\nSM\r\n\r\n". So 
>> the buffer will not be fulfilled.
>> 
> 
> The recv() operation gets buffer size as an argument.
> 
> Thus, the buffer can be fully filled with all the data copied from
> a socket buffer.  HTTP/2 clients usually don't send preface alone.
> They can send other data with the preface. 

Yes, the buffer could be full filled. HTTP/2 client will send more frames after 
the h2 preface, this is
why I introduce the ngx_http_v2_init_after_preface(ngx_event_t *rev, ngx_buf_t 
*buf).

All the remaining data in the buffer will be copied to the h2mcf->recv_buffer 
and be processed by the HTTP/2 ngx_http_v2_state_head handler.

So this will not breaks the HTTP/2 handshake.

However, if we received data is longer than the h2mcf->recv_buffer_size, the 
current patch will yield a
bad request response. 

A bad case is the client_header_buffer_size(default 1k) set larger than the 
http2_recv_buffer_size(default 265k), we will may fully filled the header_in 
buffer but cannot be copied into the h2mcf->recv_buffer,
which will cause the HTTP/2 handshake failed. But simple way to workaround this 
issue is to change the
http2_recv_buffer_size to or larger than client_header_buffer_size. It is a 
hack.

> 
> [..]
>>> +#if (NGX_HTTP_V2)
>>> +static void
>>> +ngx_http_process_h2_preface(ngx_event_t *rev)
>>> +{
>>> +size_t  len;
>>> +ngx_connection_t   *c;
>>> +ngx_http_request_t *r;
>>> +ngx_http_connection_t  *hc;
>>> +ngx_http_v2_main_conf_t*h2mcf;
>>> +
>>> +c = rev->data;
>>> +r = c->data;
>>> +hc = r->http_connection;
>>> +
>>> +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0,
>>> +"http process h2 preface");
>>> +
>>> +r->header_in->pos = r->header_in->start + 24;
>>> +
>>> +len = r->header_in->last - r->header_in->pos;
>>> +
>>> +h2mcf = ngx_http_get_module_main_conf(hc->conf_ctx, 
>>> ngx_http_v2_module);
>>> +
>>> +if (len <= h2mcf->recv_buffer_size) {
>> 
>> We check the recv buffer size before do the ngx_http_v2_init_after_preface.
>> 
> [..]
> 
> See above.  It's a bad idea to decline request with a "client sent
> invalid h2 preface" message in error log just because client has
> sent more HTTP/2 frames right after the preface.

In this path, all the valid HTTP/2 handshake will never generate a error log.

> 
> Overall, the patch looks like a hack and introduces too much
> complexity for this feature.  While I understand the reasoning,
> the proposed implementation cannot be accepted.

Could you clarify that whether is this feature not accepted or this patch?

If this feature is not needed, I will terminate this thread.

If this patch only looks like a hack, would you like offer any advice to write
code with good smell?

Thank you.

> 
> Protocol detection before starting processing HTTP/1 or HTTP/2
> could be a better way to go.
> 
>  wbr, Valentin V. Bartenev
> 
> ___
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel



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

[PATCH] HTTP/2: make http2 server support http1

[Haitao Lv]

# HG changeset patch
# User 吕海涛 
# 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;
  

Re: [PATCH] HTTP/2: make http2 server support http1

[Haitao Lv]

Hello, here is another patch(more sample) according Maxim Dounin advice.

Please offer your comment. Thanks.

diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
index 89cfe77a..71bc7b59 100644
--- a/src/http/ngx_http_request.c
+++ b/src/http/ngx_http_request.c
@@ -17,6 +17,10 @@ static ssize_t 
ngx_http_read_request_header(ngx_http_request_t *r);
 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_wait_v2_preface_handler(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,
@@ -321,7 +325,7 @@ ngx_http_init_connection(ngx_connection_t *c)
 
 #if (NGX_HTTP_V2)
 if (hc->addr_conf->http2) {
-rev->handler = ngx_http_v2_init;
+rev->handler = ngx_http_wait_v2_preface_handler;
 }
 #endif
 
@@ -377,6 +381,108 @@ ngx_http_init_connection(ngx_connection_t *c)
 }
 
 
+#if (NGX_HTTP_V2)
+static void
+ngx_http_wait_v2_preface_handler(ngx_event_t *rev)
+{
+size_t size;
+ssize_tn;
+ngx_buf_t *b;
+ngx_connection_t  *c;
+
+c = rev->data;
+
+ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0,
+"http wait h2 preface handler");
+
+if (rev->timedout) {
+ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "client timed out");
+ngx_http_close_connection(c);
+return;
+}
+
+if (c->close) {
+ngx_http_close_connection(c);
+return;
+}
+
+size = 5 /* strlen("PRI *") */;
+
+b = c->buffer;
+
+if (b == NULL) {
+b = ngx_create_temp_buf(c->pool, size);
+if (b == NULL) {
+ngx_http_close_connection(c);
+return;
+}
+
+c->buffer = b;
+
+} else if (b->start == NULL) {
+
+b->start = ngx_palloc(c->pool, size);
+if (b->start == NULL) {
+ngx_http_close_connection(c);
+return;
+}
+
+b->pos = b->start;
+b->last = b->start;
+b->end = b->last + size;
+}
+
+n = c->recv(c, b->last, size);
+
+if (n == NGX_AGAIN) {
+
+if (!rev->timer_set) {
+ngx_add_timer(rev, c->listening->post_accept_timeout);
+ngx_reusable_connection(c, 1);
+}
+
+if (ngx_handle_read_event(rev, 0) != NGX_OK) {
+ngx_http_close_connection(c);
+return;
+}
+
+/*
+ * We are trying to not hold c->buffer's memory for an idle connection.
+ */
+
+if (ngx_pfree(c->pool, b->start) == NGX_OK) {
+b->start = NULL;
+}
+
+return;
+}
+
+if (n == NGX_ERROR) {
+ngx_http_close_connection(c);
+return;
+}
+
+if (n == 0) {
+ngx_log_error(NGX_LOG_INFO, c->log, 0,
+  "client closed connection");
+ngx_http_close_connection(c);
+return;
+}
+
+b->last += n;
+
+if (b->last == b->end) {
+if (ngx_strncmp(b->start, "PRI *", 5) == 0) {
+ngx_http_v2_init_with_buf(rev, b);
+} else {
+rev->handler = ngx_http_wait_request_handler;
+ngx_http_wait_request_handler(rev);
+}
+}
+}
+#endif
+
+
 static void
 ngx_http_wait_request_handler(ngx_event_t *rev)
 {
@@ -430,6 +536,21 @@ ngx_http_wait_request_handler(ngx_event_t *rev)
 b->pos = b->start;
 b->last = b->start;
 b->end = b->last + size;
+} else {
+
+p = ngx_palloc(c->pool, size);
+if (p == NULL) {
+ngx_http_close_connection(c);
+return;
+}
+
+n = b->last - b->start;
+ngx_memcpy(p, b->start, n);
+
+b->start = p;
+b->pos = b->start;
+b->last = b->start + n;
+b->end = b->last + size;
 }
 
 n = c->recv(c, b->last, size);
diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c
index d9df0f90..a990c96f 100644
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -231,6 +231,14 @@ static ngx_http_v2_parse_header_t  
ngx_http_v2_parse_headers[] = {
 void
 ngx_http_v2_init(ngx_event_t *rev)
 {
+ngx_http_v2_init_with_buf(rev, NULL);
+}
+
+
+void
+ngx_http_v2_init_with_buf(ngx_event_t *rev, ngx_buf_t *buf)
+{
+size_t size;
 ngx_connection_t  *c;
 ngx_pool_cleanup_t*cln;
 ngx_http_connection_t *hc;
@@ -262,6 +270,17 @@ ngx_http_v2_init(ngx_event_t *rev)
 return;
 }
 
+if (buf != NULL) {
+size = buf->last - buf->start;
+
+if (size > h2mcf->recv_buffer_size) {
+size = h2mcf->recv_buffer_size;
+}
+
+ngx_memcpy(h2mcf->recv_buffer, buf->start, size);
+h2c->state.buffer_used = size;
+}
+
 h2c->connection = c;