Re: [PATCH] HTTP/2: make http2 server support http1
hello? 发自我的 iPhone > 在 2018年4月2日,08:28,Haitao Lv 写道: > > Any body is here? > >> On Mar 21, 2018, at 11:36, Haitao Lv 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_handler; >> +ngx_http_wait_request_handler(rev); >> +} >> +} >> +} >> +#endif >> + >> + >> static void >> ngx_http_wait_request_handler(ngx_event_t *rev) >> { >> @@ -393,6 +522,7 @@ ngx_http_wait_request_handler(ngx_event_t *rev) >>ngx_http_core_srv_conf_t *cscf; >> >>c = rev->data; >> +n = NGX_AGAIN; >> >>ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "http wait request >> handler"); >> >> @@ -434,9 +564,27 @@
Re: [PATCH] HTTP/2: make http2 server support http1
Any body is here? > On Mar 21, 2018, at 11:36, Haitao Lvwrote: > > 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_handler; > +ngx_http_wait_request_handler(rev); > +} > +} > +} > +#endif > + > + > static void > ngx_http_wait_request_handler(ngx_event_t *rev) > { > @@ -393,6 +522,7 @@ ngx_http_wait_request_handler(ngx_event_t *rev) > ngx_http_core_srv_conf_t *cscf; > > c = rev->data; > +n = NGX_AGAIN; > > ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "http wait request > handler"); > > @@ -434,9 +564,27 @@ 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 ==
Re: [PATCH] HTTP/2: make http2 server support http1
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_handler; +ngx_http_wait_request_handler(rev); +} +} +} +#endif + + static void ngx_http_wait_request_handler(ngx_event_t *rev) { @@ -393,6 +522,7 @@ ngx_http_wait_request_handler(ngx_event_t *rev) ngx_http_core_srv_conf_t *cscf; c = rev->data; +n = NGX_AGAIN; ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "http wait request handler"); @@ -434,9 +564,27 @@ 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->pos; +ngx_memcpy(p, b->pos, n); +ngx_pfree(c->pool, b->start); + +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); +
Re: [PATCH] HTTP/2: make http2 server support http1
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
Re: [PATCH] HTTP/2: make http2 server support http1
ping @wbr > On Mar 13, 2018, at 01:11, Valentin V. Bartenevwrote: > > 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 写道: >>> >>> 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; >>> +} >>> + >>> +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) { >>> +/* b will be freed in >>> ngx_http_v2_init/ngx_http_wait_request_handler */ >>> + >>> +if (ngx_strncmp(b->start, preface, size) == 0) { >>> +ngx_http_v2_init(rev); >>> +} 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 +538,22 @@ 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; >>> +} >>> +
Re: [PATCH] HTTP/2: make http2 server support http1
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写道: > > > > 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; > > +} > > + > > +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) { > > +/* b will be freed in > > ngx_http_v2_init/ngx_http_wait_request_handler */ > > + > > +if (ngx_strncmp(b->start, preface, size) == 0) { > > +ngx_http_v2_init(rev); > > +} 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 +538,22 @@ 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); > > +
Re: [PATCH] HTTP/2: make http2 server support http1
Is there any one who would like to review this patch? 发自我的 iPhone > 在 2018年3月8日,08:42,Haitao Lv写道: > > 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; > +} > + > +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) { > +/* b will be freed in ngx_http_v2_init/ngx_http_wait_request_handler > */ > + > +if (ngx_strncmp(b->start, preface, size) == 0) { > +ngx_http_v2_init(rev); > +} 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 +538,22 @@ 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); > +ngx_pfree(c->pool, b->start); > + > +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..e36bf382 100644 > --- a/src/http/v2/ngx_http_v2.c > +++ b/src/http/v2/ngx_http_v2.c > @@ -231,6 +231,8 @@ static ngx_http_v2_parse_header_t > ngx_http_v2_parse_headers[] = { > void > ngx_http_v2_init(ngx_event_t *rev) > { > +size_t
Re: [PATCH] HTTP/2: make http2 server support http1
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; +} + +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) { +/* b will be freed in ngx_http_v2_init/ngx_http_wait_request_handler */ + +if (ngx_strncmp(b->start, preface, size) == 0) { +ngx_http_v2_init(rev); +} 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 +538,22 @@ 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); +ngx_pfree(c->pool, b->start); + +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..e36bf382 100644 --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -231,6 +231,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; @@ -262,6 +264,23 @@ ngx_http_v2_init(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; +} + +
Re: [PATCH] HTTP/2: make http2 server support http1
Here is a more simple patch. And the temp buffer has also been freed. Please make your comments. Thanks. diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c index 89cfe77a..d97952bc 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, 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) { +/* b will be freed in ngx_http_v2_init/ngx_http_wait_request_handler */ + +if (ngx_strncmp(b->start, preface, size) == 0) { +ngx_http_v2_init(rev); +} 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 +538,22 @@ 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); +ngx_pfree(c->pool, b->start); + +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..e36bf382 100644 --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -231,6 +231,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; @@ -262,6 +264,23 @@ ngx_http_v2_init(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);
Re: [PATCH] HTTP/2: make http2 server support http1
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;
Re: [PATCH] HTTP/2: make http2 server support http1
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
Re: [PATCH] HTTP/2: make http2 server support http1
Hello wbr, Thank you for reviewing this patch. > On Mar 5, 2018, at 23:06, Valentin V. Bartenevwrote: > > 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
Re: [PATCH] HTTP/2: make http2 server support http1
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. [..] > > +#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. [..] > > +#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. 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. 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
Re: [PATCH] HTTP/2: make http2 server support http1
> On Mar 5, 2018, at 17:02, Ruslan Ermilovwrote: > > 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
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. ___ 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
@wbr > On Mar 4, 2018, at 13:49, Haitao Lvwrote: > > 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; > -} > - 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. > if ((ch < 'A' || ch > 'Z') && ch != '_' && ch != '-') { > return NGX_HTTP_PARSE_INVALID_METHOD; > } > @@ -174,6 +177,13 @@ ngx_http_parse_request_line(ngx_http_request_t *r, > ngx_buf_t *b) > break; > } > > +#if (NGX_HTTP_V2) > +
Re: [PATCH] HTTP/2: make http2 server support http1
> On Mar 4, 2018, at 21:00, Valentin V. Bartenevwrote: > > 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
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; -} - if ((ch < 'A' || ch > 'Z') && ch != '_' && ch != '-') { return NGX_HTTP_PARSE_INVALID_METHOD; } @@ -174,6 +177,13 @@ ngx_http_parse_request_line(ngx_http_request_t *r, ngx_buf_t *b) break; } +#if (NGX_HTTP_V2) +if (ngx_str3_cmp(m, 'P', 'R', 'I', ' ')) { +r->method = NGX_HTTP_PRI; +break; +} +#endif + break; case 4: @@ -267,6 +277,14 @@ ngx_http_parse_request_line(ngx_http_request_t *r, ngx_buf_t *b) } state = sw_spaces_before_uri; + +#if (NGX_HTTP_V2) +if (r->method & NGX_HTTP_PRI) { +
Re: [PATCH] HTTP/2: make http2 server support http1
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 Lvwrote: > > # 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 @@ >
Re: [PATCH] HTTP/2: make http2 server support http1
On Friday 02 March 2018 15:53:07 Haitao Lv wrote: > # 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 > [..] It doesn't look like a useful feature. Could you please explain the use cases? > 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 [..] > +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; > +} [..] What if the received data is bigger than h2mcf->recv_buffer? wbr, Valentin V. Bartenev ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: make http2 server support http1
# 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;