This repository has been archived by the owner. It is now read-only.

Hangs when request contains "Connection: close" header #418

Closed
Daniel15 opened this Issue Nov 22, 2015 · 14 comments

Comments

Projects
None yet
9 participants
@Daniel15
Copy link
Contributor

Daniel15 commented Nov 22, 2015

Kestrel hangs when the "Connection: close" request header is provided:

screen shot 2015-11-22 at 3 44 41 pm


Original post:

I'm trying to upgrade my site from Beta 6 to RC 1. I'm encountering an issue whereby hitting Kestrel directly (ie. curl http://localhost:5000/foo) works fine, but hitting it via an Nginx proxy results in the request hanging and no response being received. Interestingly, if I kill Kestrel, the request does complete. The same configuration worked fine with Beta 6.

Relevant parts of the Nginx config:

    location / {
        # Pass requests for unknown files to ASP.NET
        try_files $uri $uri/index.htm @aspnet;
    }

    location @aspnet {
        proxy_pass http://127.0.0.1:5000;
        proxy_set_header Host $host;
    }

The site is my personal site, code is on Github: https://github.com/Daniel15/Website

@Daniel15 Daniel15 changed the title RC 1 Kestrel non-responsive when proxying via Nginx Hangs when request contains "Connection: close" header Nov 22, 2015

@Daniel15

This comment has been minimized.

Copy link
Contributor

Daniel15 commented Nov 22, 2015

I worked out that Nginx is sending a Connection: close header, which is what's breaking it. Updated the issue to clarify.

@Daniel15

This comment has been minimized.

Copy link
Contributor

Daniel15 commented Nov 22, 2015

Looks like this is fixed by e4fd91b cc @cesarbs - Is there a nightly build of Kestrel I can use that includes this fix?

@Daniel15 Daniel15 closed this Nov 22, 2015

@khellang

This comment has been minimized.

Copy link
Contributor

khellang commented Nov 22, 2015

@Daniel15

This comment has been minimized.

Copy link
Contributor

Daniel15 commented Nov 22, 2015

Thanks @khellang! It looks like I can't just use the latest Kestrel along with RC1 versions of everything else, as it has RC2 dependencies. I might just download the RC1 code and manually patch the fix in (assuming it patches cleanly on RC1)

@Daniel15

This comment has been minimized.

Copy link
Contributor

Daniel15 commented Nov 23, 2015

Here's the patch I applied on RC1 to fix this:

diff --git a/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs b/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs
index 8f1c804..70890e8 100644
--- a/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs
+++ b/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs
@@ -198,10 +198,13 @@ public Task Stop()

                             await ProduceEnd();

-                            while (await RequestBody.ReadAsync(_nullBuffer, 0, _nullBuffer.Length) != 0)
-                            {
-                                // Finish reading the request body in case the app did not.
-                            }
+                           if (_keepAlive)
+                           {
+                               while (await RequestBody.ReadAsync(_nullBuffer, 0, _nullBuffer.Length) != 0)
+                               {
+                                   // Finish reading the request body in case the app did not.
+                               }
+                           }
                         }

                         terminated = !_keepAlive;
diff --git a/src/Microsoft.AspNet.Server.Kestrel/project.json b/src/Microsoft.AspNet.Server.Kestrel/project.json
index cccd1ac..c21b567 100644
--- a/src/Microsoft.AspNet.Server.Kestrel/project.json
+++ b/src/Microsoft.AspNet.Server.Kestrel/project.json
@@ -1,5 +1,5 @@
 {
-  "version": "1.0.0-rc1-final",
+  "version": "1.0.0-rc1-final-patched406",
   "description": "ASP.NET 5 cross platform development web server.",
   "repository": {
     "type": "git",

I'll replace my patched version once RC2 is out 👍

@seburgi

This comment has been minimized.

Copy link

seburgi commented Dec 3, 2015

I'm so happy I found this, I was bumping into this problem for some hours.

For now I'm adding the following lines to my nginx config, so it doesn't send "Connection: close", when RC2 is out I will remove them again:

location / {
    ....
    proxy_set_header Connection "";
    proxy_http_version 1.1;
}
@aggieben

This comment has been minimized.

Copy link

aggieben commented Dec 30, 2015

Just to clarify @seburgi's comment - I tried only removing the Connection header, but that doesn't work. Both lines are required.

@buraktamturk

This comment has been minimized.

Copy link

buraktamturk commented Apr 21, 2016

@Daniel15 Hi, have you uploaded "1.0.0-rc1-final-patched406" to anywhere? The workaround on nginx does not work for websocket requests. And I don't really know uploading dnx stuff on nuget.

I also tried something like this with no luck:

set $cnn "";
if ($http_connection = Upgrade) {
set $cnn $http_connection;
}
proxy_set_header Connection $cnn;

This problem seems to be happens when Connection header is set to Upgrade also, which is required for websocket.

@Daniel15

This comment has been minimized.

Copy link
Contributor

Daniel15 commented Apr 21, 2016

@buraktamturk - No, sorry, I didn't upload my patched version anywhere and I stopped using my patched version. You should be able to clone this repo, checkout the RC1 tag, apply the patch, and compile it. You can then add the local build directory as a package source in Visual Studio.

@buraktamturk

This comment has been minimized.

Copy link

buraktamturk commented Apr 21, 2016

@Daniel15 Let me give a try :D

@Yantrio

This comment has been minimized.

Copy link

Yantrio commented Aug 10, 2016

It looks like this hasn't been fixed, I just had to add

            proxy_set_header Connection "";
            proxy_http_version 1.1;

to my nginx config for a reverse proxy :( Any clue on if this was supposed to be fixed or not?

@cesarbs

This comment has been minimized.

Copy link
Contributor

cesarbs commented Aug 10, 2016

Possibly related: #1041 cc @halter73

@halter73

This comment has been minimized.

Copy link
Member

halter73 commented Aug 16, 2016

@Yantrio @jchannon I just tried this scenario with Kestrel 1.0.0 using our default template project. I hit both the index (a razor view) and a static file. Both cases worked fine. Can you provide more details about the issues your still seeing in 1.0.0?

The razor view does not set a content-length and Kestrel doesn't chunk the response because that isn't supported in response to HTTP/1.0 requests. The response simply ends when the response is closed which is the default behavior for HTTP/1.0.

Since Kestrel is responding to the 1.0 request with a 1.1 response, Kestrel should be sending the Connection: close header which it's not. This was fixed by #1043. Nginx then modifies the response to use Transfer-Encoding: chunked and Connection: keep-alive assuming the original non-proxied request was made using HTTP/1.1. Otherwise, nginx will set Connection: close.

I tested using nginx/1.6.2 curl 7.38.0 and curl 7.44.0

server {
    listen 80;
    location / {
        proxy_pass http://localhost:5000;
        proxy_set_header Host $host;
    }
}

Output:

$ curl http://nginxhost/favicon.ico -v --header 'Connection: close'
*   Trying nginxhost...
* Connected to nginxhost (nginxhost) port 80 (#0)
> GET /favicon.ico HTTP/1.1
> Host: nginxhost
> User-Agent: curl/7.44.0
> Accept: */*
> Connection: close
>
< HTTP/1.1 200 OK
< Server: nginx/1.6.2
< Date: Tue, 16 Aug 2016 19:09:25 GMT
< Content-Type: image/x-icon
< Content-Length: 32038
< Connection: close
< Last-Modified: Tue, 21 Jun 2016 09:44:42 GMT
< Accept-Ranges: bytes
< ETag: "1d1cba1890ddc26"
<

<A bunch of bytes here>

 Closing connection 0


$ curl hhttp://nginxhost/ -v --header 'Connection: close'
*   Trying nginxhost...
* Connected to nginxhost (nginxhost) port 80 (#0)
> GET / HTTP/1.1
> Host: nginxhost
> User-Agent: curl/7.44.0
> Accept: */*
> Connection: close
>
< HTTP/1.1 200 OK
< Server: nginx/1.6.2
< Date: Tue, 16 Aug 2016 19:10:51 GMT
< Content-Type: text/html; charset=utf-8
< Transfer-Encoding: chunked
< Connection: close
<

<A bunch of HTML here>

* Closing connection 0


$ curl http://nginxhost/favicon.ico -v
*   Trying nginxhost...
* Connected to nginxhost (nginxhost) port 80 (#0)
> GET /favicon.ico HTTP/1.1
> Host: nginxhost
> User-Agent: curl/7.44.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Server: nginx/1.6.2
< Date: Tue, 16 Aug 2016 19:12:09 GMT
< Content-Type: image/x-icon
< Content-Length: 32038
< Connection: keep-alive
< Last-Modified: Tue, 21 Jun 2016 09:44:42 GMT
< Accept-Ranges: bytes
< ETag: "1d1cba1890ddc26"
<

<A bunch of bytes here>

 Closing connection 0


$ curl http://nginxhost/ -v
*   Trying nginxhost...
* Connected to nginxhost (nginxhost) port 80 (#0)
> GET / HTTP/1.1
> Host: nginxhost
> User-Agent: curl/7.44.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Server: nginx/1.6.2
< Date: Tue, 16 Aug 2016 19:12:52 GMT
< Content-Type: text/html; charset=utf-8
< Transfer-Encoding: chunked
< Connection: keep-alive
<

<A bunch of HTML here>

* Closing connection 0


$ curl http://nginxhost/favicon.ico -v -0
*   Trying nginxhost...
* Connected to nginxhost (nginxhost) port 80 (#0)
> GET /favicon.ico HTTP/1.0
> Host: nginxhost
> User-Agent: curl/7.44.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Server: nginx/1.6.2
< Date: Tue, 16 Aug 2016 19:14:52 GMT
< Content-Type: image/x-icon
< Content-Length: 32038
< Connection: close
< Last-Modified: Tue, 21 Jun 2016 09:44:42 GMT
< Accept-Ranges: bytes
< ETag: "1d1cba1890ddc26"
<

<A bunch of bytes here>

 Closing connection 0

$ curl http://nginxhost/ -v -0
*   Trying nginxhost...
* Connected to nginxhost (nginxhost) port 80 (#0)
> GET / HTTP/1.0
> Host: nginxhost
> User-Agent: curl/7.44.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Server: nginx/1.6.2
< Date: Tue, 16 Aug 2016 19:15:29 GMT
< Content-Type: text/html; charset=utf-8
< Connection: close
<

<A bunch of HTML here>

* Closing connection 0

I even tried testing sending an app data that wasn't expecting/reading it to ensure there wasn't a reemergence of the bug fixed in #275

$ curl http://nginxhost/ -v -0 --data "param1=value1"
*   Trying nginxhost...
* Connected to nginxhost (nginxhost) port 80 (#0)
> POST / HTTP/1.0
> Host: nginxhost
> User-Agent: curl/7.44.0
> Accept: */*
> Content-Length: 13
> Content-Type: application/x-www-form-urlencoded
>
* upload completely sent off: 13 out of 13 bytes
< HTTP/1.1 200 OK
< Server: nginx/1.6.2
< Date: Tue, 16 Aug 2016 19:17:35 GMT
< Content-Type: text/html; charset=utf-8
< Connection: close
<


<A bunch of HTML here>

* Closing connection 0

I also logged the raw request data on the server side to verify that nginx was indeed sending non keep-alive HTTP/1.0 requests.

@sphiecoh

This comment has been minimized.

Copy link

sphiecoh commented Oct 28, 2016

Upgrading to 1.1.0-preview1-final fixes the issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.