Opened 5 years ago

Closed 2 years ago

#1743 closed defect (fixed)

Can't flush HTTP response header under TLS+HTTP2

Reported by: mrpre@… Owned by:
Priority: minor Milestone:
Component: nginx-module Version: 1.14.x
Keywords: http2 Cc:
uname -a: centos7 CST 2017 x86_64 x86_64 x86_64 GNU/Linux
nginx -V: --with-stream --with-stream_ssl_module --with-http_ssl_module --with-http_v2_module

Description


I want to send HTTP response header first before data is sent to client.
so I do like this:

/*Want to response Header and flush*/
ngx_http_idle_send_header(r);
/*ngx_http_send_special doesn't work under HTTP2 but works under HTTP1*/
ngx_http_send_special(r, NGX_HTTP_FLUSH);
......
/*send Data later*/
ngx_http_output_filter(r, &out);

I had already set postpone_output to 0, and then I found HTTP response header was cached in ssl_buffer( in function ngx_ssl_send_chain ).

Under HTTP 1, function ngx_ssl_send_chain will set c->buffered which c was the real connection and then we call ngx_http_send_special->...->ngx_http_write_filter->ngx_ssl_send_chain, the header was sent on network. That was I want.

Under HTTP 2, function ngx_ssl_send_chain will set c->buffered which c was the real connection but when ngx_http_send_special->...->ngx_http_write_filter was called, ngx_ssl_send_chain wasn't called, because the c in ngx_http_write_filter was the H2 fake connection and c->buffered was not set

    if (size == 0
        && !(c->buffered & NGX_LOWLEVEL_BUFFERED)
        && !(last && c->need_last_buf))
    {
        /*return here*/
        if (last || flush || sync) {
            for (cl = r->out; cl; /* void */) {
                ln = cl;
                cl = cl->next;
                ngx_free_chain(r->pool, ln);
            }

            r->out = NULL;
            c->buffered &= ~NGX_HTTP_WRITE_BUFFERED;

            return NGX_OK;
        }

        ngx_log_error(NGX_LOG_ALERT, c->log, 0,
                      "the http output chain is empty");

        ngx_debug_point();

        return NGX_ERROR;
    }

What should I do if I want to flush HTTP response header into network before data is sent ?

Change History (5)

comment:1 by mrpre@…, 5 years ago

Update
What I want is that DATA frame is delayed one minute.

/*HEAD frame first*/
ngx_http_send_header(r);

/*NGX_HTTP_FLUSH doesn't wrok, HEAD frame still cached in ssl buffer*/
ngx_http_send_special(r, NGX_HTTP_FLUSH);

/*This action will flush HEAD frame which means HEAD frame and DATA frame are sent by one tcp segment*/
ngx_http_output_filter(r, &out);

comment:2 by Maxim Dounin, 4 years ago

Keywords: http2 added

comment:3 by Maxim Dounin, 2 years ago

Status: newaccepted

Thanks for reporting this. I'm able to reproduce this with the following test configuration using embedded perl:

location / {
    perl 'sub {
        my $r = shift;
        $r->sleep(1000, \&header);
        return OK;
        
        sub header {
            my $r = shift;
            $r->send_http_header("text/html");
            return OK if $r->header_only;
            $r->flush();
            $r->sleep(1000, \&done);
            return OK;
        }

        sub done {
            my $r = shift;
            $r->print("foo\n");
            return OK;
        }
    }';
}

and curl:

$ curl -ik --http2 https://127.0.0.1:8443/
[2 second pause here]
HTTP/2 200 
server: nginx/1.21.6
date: Wed, 26 Jan 2022 17:10:04 GMT
content-type: text/html

foo

vs. with HTTP/1.x:

$ curl -ik --http1.1 https://127.0.0.1:8443/
[1 second pause here]
HTTP/1.1 200 OK
Server: nginx/1.21.6
Date: Wed, 26 Jan 2022 17:11:45 GMT
Content-Type: text/html
Transfer-Encoding: chunked
Connection: keep-alive

[1 second pause here]
foo

Note that there is an initial delay before sending headers: this is to avoid unrelated flushes during HTTP/2 connection establishment.

With HTTP/1.1, responses headers are properly flushed and sent to the client immediately when generated by the code, as per $r->flush(). With HTTP/2, flush is ignored and response headers are only sent with the response body when the $r->sleep() timer expires.

It does not seem to be possible to provide stream's fc->buffered to make existing write filter code behave correctly for HTTP/2, since we do not know if main connection's c->buffered corresponds to the particular stream or not. And blindly mapping c->buffered to fc->buffered might prevent request finalization due to sending data on another stream.

The most simple fix seems to always flush headers, similarly to what we already do with data (since 5549:39d7eef2e332, "SPDY: protocol implementation switched to spdy/3.1"). This, however, will result in additional SSL_write() call and extra packets being sent in case of typical static responses, which are used to send headers along with the response body. Patch as follows:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1643224220 -10800
#      Wed Jan 26 22:10:20 2022 +0300
# Node ID 7b23cd6bd85fbf366ef2054df1d27bb1329f0657
# Parent  56ead48cfe885e8b89b30017459bf621b21d95f5
HTTP/2: response headers are always flushed now (ticket #1743).

Response headers can be buffered in the SSL buffer.  But stream's fake
connection buffered flag did not reflect this, so any attempts to flush
the buffer without sending additional data were stopped by the write filter.

It does not seem to be possible to reflect this in fc->buffered though, as
we never known if main connection's c->buffered corresponds to the particular
stream or not.  As such, fc->buffered might prevent request finalization
due to sending data on some other stream.

Fix is to always flush headers, similarly to what we already do with DATA
frames.

diff --git a/src/http/v2/ngx_http_v2_filter_module.c b/src/http/v2/ngx_http_v2_filter_module.c
--- a/src/http/v2/ngx_http_v2_filter_module.c
+++ b/src/http/v2/ngx_http_v2_filter_module.c
@@ -1167,6 +1167,7 @@ ngx_http_v2_create_headers_frame(ngx_htt
             continue;
         }
 
+        b->flush = 1;
         b->last_buf = fin;
         cl->next = NULL;
         frame->last = cl;

Alternatively, empty flush buffers might be passed by the write filter to c->send_chain(), similarly to how similar problem with empty last buffers is handled for HTTP/2 (per the c->need_last_buf flag). Below is an alternative patch:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1643225036 -10800
#      Wed Jan 26 22:23:56 2022 +0300
# Node ID f76e0cf8e525996563e5f0092fa48a4fee873e93
# Parent  56ead48cfe885e8b89b30017459bf621b21d95f5
HTTP/2: made it possible to flush response headers (ticket #1743).

Response headers can be buffered in the SSL buffer.  But stream's fake
connection buffered flag did not reflect this, so any attempts to flush
the buffer without sending additional data were stopped by the write filter.

It does not seem to be possible to reflect this in fc->buffered though, as
we never known if main connection's c->buffered corresponds to the particular
stream or not.  As such, fc->buffered might prevent request finalization
due to sending data on some other stream.

Fix is to implement handling of flush buffers when c->need_last_buf is set,
similarly to the existing last buffer handling.

diff --git a/src/http/ngx_http_write_filter_module.c b/src/http/ngx_http_write_filter_module.c
--- a/src/http/ngx_http_write_filter_module.c
+++ b/src/http/ngx_http_write_filter_module.c
@@ -227,7 +227,7 @@ ngx_http_write_filter(ngx_http_request_t
 
     if (size == 0
         && !(c->buffered & NGX_LOWLEVEL_BUFFERED)
-        && !(last && c->need_last_buf))
+        && !((last || flush) && c->need_last_buf))
     {
         if (last || flush || sync) {
             for (cl = r->out; cl; /* void */) {
diff --git a/src/http/v2/ngx_http_v2_filter_module.c b/src/http/v2/ngx_http_v2_filter_module.c
--- a/src/http/v2/ngx_http_v2_filter_module.c
+++ b/src/http/v2/ngx_http_v2_filter_module.c
@@ -1815,7 +1815,11 @@ ngx_http_v2_waiting_queue(ngx_http_v2_co
 static ngx_inline ngx_int_t
 ngx_http_v2_filter_send(ngx_connection_t *fc, ngx_http_v2_stream_t *stream)
 {
-    if (stream->queued == 0) {
+    ngx_connection_t  *c;
+
+    c = stream->connection->connection;
+
+    if (stream->queued == 0 && !c->buffered) {
         fc->buffered &= ~NGX_HTTP_V2_BUFFERED;
         return NGX_OK;
     }

I tend to think that proper flush handling as in the second patch is a better approach, which is also in line with HTTP/1.x code. Further, it will allow us to remove automatic flushing of data frames if we'll decide to do this eventually (and/or eventually add automatic flushing of header frames).

Review and testing appreciated.

comment:4 by Maxim Dounin <mdounin@…>, 2 years ago

In 8006:32b0ba4855a6/nginx:

HTTP/2: made it possible to flush response headers (ticket #1743).

Response headers can be buffered in the SSL buffer. But stream's fake
connection buffered flag did not reflect this, so any attempts to flush
the buffer without sending additional data were stopped by the write filter.

It does not seem to be possible to reflect this in fc->buffered though, as
we never known if main connection's c->buffered corresponds to the particular
stream or not. As such, fc->buffered might prevent request finalization
due to sending data on some other stream.

Fix is to implement handling of flush buffers when the c->need_flush_buf
flag is set, similarly to the existing last buffer handling. The same
flag is now used for UDP sockets in the stream module instead of explicit
checking of c->type.

comment:5 by Maxim Dounin, 2 years ago

Resolution: fixed
Status: acceptedclosed

Fix committed. Thanks for reporting this.

Note: See TracTickets for help on using tickets.