SPDY and Proxy Cache prematurely closes connections

[Phil Sweeney]

As originally reported via mailing list/forum (a while ago):
​http://forum.nginx.org/read.php?2,233497,241193#msg-241193

When SPDY and proxy cache are both being used, nginx will prematurely close the connection to avoid sending the body to the client. However with SPDY, there could be other items still to send, so this causes empty responses. In my case, css/js/images come back empty to the browser occasionally.

Debug logs have been provided to Valentin V. Bartenev, who stated in April:

"Well, from what I've examined from your debug logs, it looks like a known feature of caching mechanism, that closes connection to prevent sending body to a client, while the whole response is receiving from upstream. It's not a big deal in http, but turns into serious issue with spdy."

"The problem appears when the whole page is requested from upstream for caching purposes, but the client should not get its body (i.e. 304 response, or HEAD request)."

Didn't originally file a ticket here because Valentin hoped that rewrites going on at the time for the upstream code might fix this, but looks like it did not.

[Valentin V. Bartenev]

This patch should fix the problem:

src/http/ngx_http_upstream.c

@@ -66 +66 @@
 static void
     ngx_http_upstream_process_non_buffered_request(ngx_http_request_t *r,
     ngx_uint_t do_write);
+static ngx_int_t ngx_http_upstream_output_filter(ngx_http_request_t *r,
+    ngx_chain_t *in);
 static ngx_int_t ngx_http_upstream_non_buffered_filter_init(void *data);
 static ngx_int_t ngx_http_upstream_non_buffered_filter(void *data,
     ssize_t bytes);
@@ -2216 +2218 @@
 
     c = r->connection;
 
-    if (r->header_only) {
-
-        if (u->cacheable || u->store) {
-
-            if (ngx_shutdown_socket(c->fd, NGX_WRITE_SHUTDOWN) == -1) {
-                ngx_connection_error(c, ngx_socket_errno,
-                                     ngx_shutdown_socket_n " failed");
-            }
-
-            r->read_event_handler = ngx_http_request_empty_handler;
-            r->write_event_handler = ngx_http_request_empty_handler;
-            c->error = 1;
-
-        } else {
-            ngx_http_upstream_finalize_request(r, u, rc);
-            return;
-        }
+    if (r->header_only && !(u->cacheable || u->store)) {
+        ngx_http_upstream_finalize_request(r, u, rc);
+        return;
     }
 
     if (r->request_body && r->request_body->temp_file) {
@@ -2384 +2372 @@
 
     p = u->pipe;
 
-    p->output_filter = (ngx_event_pipe_output_filter_pt) ngx_http_output_filter;
+    p->output_filter =
+        (ngx_event_pipe_output_filter_pt) ngx_http_upstream_output_filter;
+
     p->output_ctx = r;
     p->tag = u->output.tag;
     p->bufs = u->conf->bufs;
@@ -2843 +2833 @@
         if (do_write) {
 
             if (u->out_bufs || u->busy_bufs) {
-                rc = ngx_http_output_filter(r, u->out_bufs);
+                rc = ngx_http_upstream_output_filter(r, u->out_bufs);
 
                 if (rc == NGX_ERROR) {
                     ngx_http_upstream_finalize_request(r, u, NGX_ERROR);
@@ -2943 +2933 @@
 
 
 static ngx_int_t
+ngx_http_upstream_output_filter(ngx_http_request_t *r, ngx_chain_t *in)
+{
+
+    if (!r->header_only) {
+        return ngx_http_output_filter(r, in);
+    }
+
+    while (in) {
+        in->buf->pos = in->buf->last;
+        in = in->next;
+    }
+
+    return NGX_OK;
+}
+
+
+static ngx_int_t
 ngx_http_upstream_non_buffered_filter_init(void *data)
 {
     return NGX_OK;

Feel free to use it.

[Phil Sweeney]

Thanks for that. I have tried it against 1.5.10 and it does appear to solve the issue!

Do you expect to apply this to mainline or does it require further work before that?

[Valentin V. Bartenev]

Replying to Phil Sweeney <philcollect@gmail.com>:

Do you expect to apply this to mainline or does it require further work before that?

Yes, it needs more work. But not because it does something wrong, the patch is correct and shouldn't introduce any bugs. Just the way it solves the problem hasn't been liked by colleagues.

[Ba Duong]

This bug makes me 3 days to investigate and search for solution to use SPDY with proxy cache.
The issue occurs if use

proxy_cache_bypass $no_cache;

or cached items deleted on server while use F5 in browsers. Only the first request is response (GET 304 status) and error with subsequent requests.
No problem if I remove SPDY.

I tested the patch of Valentin V. Bartenev on 1.5.10 and it work for me.
Thanks.

Update: Sorry, but today I got an error in Chrome 33 Dev tool when I post several wordpress comments:
net::ERR_SPDY_PROTOCOL_ERROR.

[sedaj2@…]

Hello,

it this problem fixed in 1.5.13?

[Valentin V. Bartenev]

Replying to www.google.com/accounts/o8/id?id=AItOawlDriEnv-y6Xg9DA3Xg0k8r-h-p-wHri4w:

it this problem fixed in 1.5.13?

No. The patch above is still relevant.

[Harry Tuttle]

hi,

is the vuln and the patch valid for the stable 1.4.7 too?

brad

[Valentin V. Bartenev]

The issue and the patch valid for 1.4.x branch too.

[pavel stano]

Patch work also for 1.6.
If it is not yet fixed information should be in documentation or somewhere.

I encountered also second problem with this issue. With limit_conn it looks like connections are not decreased when they are prematurely closed. So you can see in log that connections are limited on spdy but they are not over limit.

[Maxim Dounin]

Fixed by 35990c69b3ac.