Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net::ERR_CONNECTION_RESET in browser when sending large JSON body and not consuming #1479

Open
MaxGabriel opened this issue Jan 22, 2018 · 12 comments

Comments

@MaxGabriel
Copy link
Member

Cross-posted from Stackoverflow, since after some investigation I think this is a bug.

I'm sending a large JSON body (~3MB) in a production app (it's a base64 encoded file). When I send a JSON body and don't consume it (just return ()), Yesod seems to think this worked:

POST /send-json
  Accept: */*
  Status: 200 OK 0.000098s

But the browser doesn't get a response back and instead gets net::ERR_CONNECTION_RESET. I've reproduced this in Chrome, Safari and Firefox.

If I actually consume the body using requireCheckJsonBody, then the issue goes away, which makes me think this is a bug.

If this is a bug, I imagine it could be somewhere lower level than Yesod, e.g. warp/wai.

I reproduced this issue in this repo here: https://github.com/MaxGabriel/large-files. It's the yesod-simple template with just 1 additional commit: MaxGabriel/large-files@38addf7. Instead of sending a file, I just copy a string a bunch of times to create a huge JSON body (a few megabytes).

I couldn't reproduce this issue sending a large binary file in a form, but I think that may have been because the form was consumed by the Yesod app—I haven't checked that bit.

I can't reproduce this issue from curl.

Yesod is configured with:

maximumContentLength _ _ = Just (10 * 1024 * 1024) -- 10 megabytes

so receiving a large request body shouldn't be a problem.

Version Info: https://gist.github.com/MaxGabriel/10e8a999eabcc9cb566c087978f5c3ac

LTS: LTS-Haskell 9.14

@snoyberg
Copy link
Member

snoyberg commented Jan 23, 2018 via email

@MaxGabriel
Copy link
Member Author

Yes I can reproduce with just Warp:

#!/usr/bin/env stack
-- stack --resolver lts-9.14 script

{-# LANGUAGE OverloadedStrings #-}
module Main where

import Network.HTTP.Types (status200)
import Network.Wai
import Network.Wai.Handler.Warp

main = run 3000 $ \request respond -> do
  putStrLn "returning response"
  _body <- strictRequestBody request -- Comment out this line to trigger the error
  respond $ responseLBS status200 [] "Hello World"

I used the following HTML to generate the request:

<head>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script>
</head>

<body>

<script>
$(function() {
  $("#send-json").on('click', function(event) {
    var string = "";

    for (i = 0; i < 55000; i++) {
      string += "a".repeat(64)
      string += "\n"
    }

    $.ajax({
      url: 'http://localhost:3000/large-json',
      type: 'POST',
      contentType: "application/json",
      data: JSON.stringify({
        bigString: string,
      }),
      success: function (data) {
        console.log(data);
      },
      error: function (data) {
        console.log("Error: " + data);
      },
    });
  });
});
</script>
<button id="send-json">Send Large JSON</button>
</body>

(Note: to workaround CORS issues, I initially served the HTML file using python -m SimpleHTTPServer 3000, opened localhost:3000 in my browser, killed the python server, then ran the warp server. Ideally I'd just serve the file from Warp but I didn't look into that).

So far I can't reproduce with a <form>, something like:

<form action="http://localhost:3000/large-file" method="post">
	<input type="file">
	<input type="submit">
</form>

given a 6.2MB file gets the "Hello World" response delivered as a new web page.

@snoyberg
Copy link
Member

I'm not sure that this is a bug on the Warp side. I can explain what's happening. When you don't consume the request body yourself in your application, Warp tries to implement an optimization to avoid excessive bandwidth usage: it will send the response body immediately, then close the connection. You can see this with the following telnet session against the example Warp application you gave above:

$ telnet localhost 3000
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
POST / HTTP/1.1
content-length: 3000000

HTTP/1.1 200 OK
Transfer-Encoding: chunked
Date: Tue, 30 Jan 2018 08:56:24 GMT
Server: Warp/3.2.13

000B
Hello World
0

Connection closed by foreign host.

Even though I indicated that I had a 3mb request body, Warp did not wait for it, gave the response, and then closed the connection. This seems to work fine with curl, and unless I'm mistaken is allowed by the HTTP spec (though I could be wrong). It may be worth changing the behavior of Warp here, or at least providing a setting for it. CC @kazu-yamamoto for input.

@MaxGabriel
Copy link
Member Author

Ah, ok. This might be the relevant part of the spec? https://tools.ietf.org/html/rfc7230#section-6.6

Some discussion about this on the nodeJS side: nodejs/node#9085 nodejs/node#12339

This does seem somewhat edge-case, but I could definitely see situations where e.g. the server immediately rejects the request without sending a response because the client was missing certain headers.

It doesn’t affect me so I’m in no hurry about this, either.

In a car so haven’t looked too closely into this since your reply

@snoyberg
Copy link
Member

Yes, those links look correct. The only other thing I can think of here is that, perhaps, Warp is in violation by not sending a connection: close header. However, I tried setting that manually in the response, and it seemed to have no impact on browser behavior.

@kazu-yamamoto
Copy link

I don't think that I understand this perfectly. But what will happen if we use shutdown instead of close in Warp when a request body remains?

@snoyberg
Copy link
Member

snoyberg commented Jan 31, 2018 via email

@kazu-yamamoto
Copy link

If a browser gets an error of connection close while sending data, the browser has no chance to read a response from the server. If the server uses shutdown, the browser does not get an error of connection close, then it gets chance to read a response. This might change the situation.

@MaxGabriel
Copy link
Member Author

If my understanding is correct, this is what @kazu-yamamoto is referencing from the HTTP spec I linked to:

To avoid the TCP reset problem, servers typically close a connection
in stages. First, the server performs a half-close by closing only
the write side of the read/write connection. The server then
continues to read from the connection until it receives a
corresponding close by the client, or until the server is reasonably
certain that its own TCP stack has received the client's
acknowledgement of the packet(s) containing the server's last
response. Finally, the server fully closes the connection.

@kazu-yamamoto
Copy link

In the telnet example, @snoyberg did not send Connection: close in HTTP/1.1 but the following code returned False, so the connection was closed:

https://github.com/yesodweb/wai/blob/master/warp/Network/Wai/Handler/Warp/Run.hs#L455

I don't understand what the code is trying to do exactly but I guess that shutdown is more proper than close here.

@nh2
Copy link

nh2 commented Feb 23, 2018

As Kazu linked correctly, I think the issue here is exactly what I bring up in yesodweb/wai#673.

I also just commented that for nodejs in nodejs/node#12339 (comment).

When an in-flight packet arrives at the kernel of the machine running the HTTP server, after close(), the kernel will reply with TCP RST.
When the browser side receives the RST, its kernel will discard any receive buffers, even for packets that it already ACKed.
These receive buffers may contain the data you just sent before the close().
Thus the client will never see that data.

This page from 2009 describes that in detail: ​https://blog.netherlabs.nl/articles/2009/01/18/the-ultimate-so_linger-page-or-why-is-my-tcp-not-reliable

So I think that

Warp tries to implement an optimization to avoid excessive bandwidth usage: it will send the response body immediately, then close the connection

is not a legal optimisation to do. If you close() a TCP connection with unread data on it, your kernel will immediately sent TCP RST, which will blow away all data in receive buffers on the other side (even if already acknowledged with TCP ACK but not yet consumed by the userspace application).

To terminate the connection, you must

  1. Send connection: close
  2. Call shutdown() on the socket
  3. Keep read()ing from the socket until the empty string is read
  4. Call close() on the socket

Step (3) guarantees the other side has actually seen all data sent by the server. Why? Because when the other side sees connection: close, or the TCP FIN following it (sent due to shutdown(), it acts upon it by closing its side of the connection, which will send TCP FIN to the server, which will result in an empty-string read().

So I think we need to remove this optimisation.

@MaxGabriel Can you confirm whether the problem goes away if you set setMaximumBodyFlush Nothing?

@MaxGabriel
Copy link
Member Author

@nh2 Yes, I can confirm that setMaximumBodyFlush Nothing fixes it. Updated script using that:

#!/usr/bin/env stack
-- stack --resolver lts-9.14 script

{-# LANGUAGE OverloadedStrings #-}
module Main where

import Network.HTTP.Types (status200)
import Network.Wai
import Network.Wai.Handler.Warp

main = runSettings (setMaximumBodyFlush Nothing defaultSettings) $ \request respond -> do
  putStrLn "returning response"
  -- _body <- strictRequestBody request -- This line no longer necessary
  respond $ responseLBS status200 [] "Hello World"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants