Upstream: reinit upstream after reading bad response.

Previously, when connecting to a backend, if the read event handler was
called before the write event handler, and the received response triggered
a next upstream condition, then ngx_http_upstream_reinit() was not called
to clean up the old upstream context.  This had multiple implications.

For all proxy modules, since the last upstream response was not cleaned up,
it was mixed with the next upstream response.  This could result in ignoring
the second response status code, duplicate response headers or reporting
old upstream header errors.

With ngx_http_grpc_module and ngx_http_proxy_v2_module, ctx->connection
was left dangling since the object it referenced was allocated from the
last upstream connection pool, which was deleted when freeing last upstream.
This lead to use-after-free when trying to reuse this object for the next
upstream.

diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c
index 1a443993f6c3c9bf83b01348f31e4209e71650c3..74042b5ec3bebce3e7220e51776f9785e8f45e22 100644 (file)
--- a/src/http/ngx_http_upstream.c
+++ b/src/http/ngx_http_upstream.c
@@ -1672,7 +1672,7 @@ ngx_http_upstream_connect(ngx_http_request_t *r, ngx_http_upstream_t *u)
     u->writer.connection = c;
     u->writer.limit = clcf->sendfile_max_chunk;
 
-    if (u->request_sent) {
+    if (u->request_sent || u->response_received) {
         if (ngx_http_upstream_reinit(r, u) != NGX_OK) {
             ngx_http_upstream_finalize_request(r, u,
                                                NGX_HTTP_INTERNAL_SERVER_ERROR);
@@ -1709,6 +1709,7 @@ ngx_http_upstream_connect(ngx_http_request_t *r, ngx_http_upstream_t *u)
     u->request_sent = 0;
     u->request_body_sent = 0;
     u->request_body_blocked = 0;
+    u->response_received = 0;
 
     if (rc == NGX_AGAIN) {
         ngx_add_timer(c->write, u->conf->connect_timeout);
@@ -2547,6 +2548,8 @@ ngx_http_upstream_process_header(ngx_http_request_t *r, ngx_http_upstream_t *u)
         u->peer.cached = 0;
 #endif
 
+        u->response_received = 1;
+
         rc = u->process_header(r);
 
         if (rc == NGX_AGAIN) {

diff --git a/src/http/ngx_http_upstream.h b/src/http/ngx_http_upstream.h
index 3afe6e8f9c4a4ebae3cf8c1e5f81111409996cd9..6176e17b647070434b0ee636ff693c42594cc31b 100644 (file)
--- a/src/http/ngx_http_upstream.h
+++ b/src/http/ngx_http_upstream.h
@@ -412,6 +412,7 @@ struct ngx_http_upstream_s {
     unsigned                         request_body_sent:1;
     unsigned                         request_body_blocked:1;
     unsigned                         header_sent:1;
+    unsigned                         response_received:1;
 };