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 822 March 02, 2018 02:54AM

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

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

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

Haitao Lv 370 March 03, 2018 10:02PM

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

Haitao Lv 194 March 04, 2018 12:52AM

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

Haitao Lv 235 March 04, 2018 10:12PM

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

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

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

Haitao Lv 189 March 05, 2018 09:38AM

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

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

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

Haitao Lv 269 March 05, 2018 10:54AM

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

Maxim Dounin 594 March 05, 2018 02:16PM

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

Haitao Lv 261 March 05, 2018 09:22PM

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

Haitao Lv 201 March 07, 2018 07:30PM

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

Haitao Lv 165 March 07, 2018 07:44PM

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

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

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

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

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

Haitao Lv 257 March 18, 2018 01:00PM

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

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

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

Haitao Lv 200 March 20, 2018 11:38PM

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

Haitao Lv 205 April 01, 2018 08:36PM

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

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

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

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

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

Haitao Lv 459 March 04, 2018 10:06PM



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

Online Users

Guests: 99
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