Welcome! Log In Create A New Profile

Advanced

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

Valentin V. Bartenev
March 05, 2018 10:08AM
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
Subject Author Views Posted

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

Haitao Lv 554 March 02, 2018 02:54AM

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

Valentin V. Bartenev 237 March 02, 2018 12:32PM

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

Haitao Lv 218 March 03, 2018 10:02PM

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

Haitao Lv 117 March 04, 2018 12:52AM

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

Haitao Lv 105 March 04, 2018 10:12PM

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

ru@nginx.com 205 March 05, 2018 04:04AM

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

Haitao Lv 120 March 05, 2018 09:38AM

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

Valentin V. Bartenev 128 March 05, 2018 10:08AM

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

Haitao Lv 177 March 05, 2018 10:54AM

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

Maxim Dounin 284 March 05, 2018 02:16PM

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

Haitao Lv 158 March 05, 2018 09:22PM

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

Haitao Lv 107 March 07, 2018 07:30PM

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

Haitao Lv 82 March 07, 2018 07:44PM

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

吕海涛 239 March 12, 2018 12:46PM

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

Valentin V. Bartenev 149 March 12, 2018 01:12PM

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

Haitao Lv 142 March 18, 2018 01:00PM

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

Valentin V. Bartenev 101 March 20, 2018 12:04PM

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

Haitao Lv 102 March 20, 2018 11:38PM

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

Haitao Lv 100 April 01, 2018 08:36PM

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

吕海涛 113 June 13, 2018 07:42PM

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

Valentin V. Bartenev 198 March 04, 2018 08:02AM

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

Haitao Lv 261 March 04, 2018 10:06PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 110
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready