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

Response not returned when server forcefully closes connection #1232

Closed
ArminBTVS opened this issue Mar 2, 2022 · 2 comments · Fixed by #1233
Closed

Response not returned when server forcefully closes connection #1232

ArminBTVS opened this issue Mar 2, 2022 · 2 comments · Fixed by #1233

Comments

@ArminBTVS
Copy link
Contributor

ArminBTVS commented Mar 2, 2022

When using fasthttp.HostClient to send a request, a response is ignored if the remote side forcefully closes the connection (via RST flag) right after sending a response.

Instead of returning the error message: "An existing connection was forcibly closed by the remote host", I would expect the response to be read and a nil error to be returned from client.Do(req, resp).

According to the HTTP spec 8.2.2 the client SHOULD monitor the network connection for an error status while it is transmitting the request. In case of fasthttp the response is not read after writing failed due to a closed connection.

ArminBTVS added a commit to ArminBTVS/fasthttp that referenced this issue Mar 3, 2022
@erikdubbelboer
Copy link
Collaborator

erikdubbelboer commented Mar 3, 2022

I see you made some changes here: ArminBTVS@76a66e1
If those changes are working I'm willing to accept a pull request.

Also curious, do you know how net/http handles this?

@ArminBTVS
Copy link
Contributor Author

ArminBTVS commented Mar 3, 2022

@erikdubbelboer so I am still evaluating my fix a bit before I open a PR. For now it looks fine. I am not sure how exactly go solves this issue, but if you look at the unit test I wrote and replace the fasthttp client with the golang client it passes:

func TestRstConnResponseWhileSending(t *testing.T) {
	const expectedStatus = http.StatusTeapot
	const payload = "payload"

	srv, err := net.Listen("tcp", "127.0.0.1:0")
	if err != nil {
		t.Fatal(err.Error())
	}

	go func() {
		conn, err := srv.Accept()
		if err != nil {
			t.Errorf(err.Error())
		}

		// Read at least one byte of the header
		// Otherwise we would have an unsolicited response
		ioutil.ReadAll(io.LimitReader(conn, 1))

		// Respond
		conn.Write([]byte("HTTP/1.1 418 Teapot\r\n\r\n"))

		// Forcefully close connection
		err = conn.(*net.TCPConn).SetLinger(0)
		if err != nil {
			t.Errorf(err.Error())
		}
		conn.Close()
	}()

	svrUrl := "http://" + srv.Addr().String()

	httpResp, err := http.Post(svrUrl, "application/json", strings.NewReader(payload))
	if err != nil {
		t.Fatal(err.Error())
	}
	if expectedStatus != httpResp.StatusCode {
		t.Fatalf("Expected %d status code, but got %d", expectedStatus, httpResp.StatusCode)
	}
}

ArminBTVS added a commit to ArminBTVS/fasthttp that referenced this issue Mar 4, 2022
ArminBTVS added a commit to ArminBTVS/fasthttp that referenced this issue Mar 7, 2022
erikdubbelboer pushed a commit that referenced this issue Mar 14, 2022
* Read response when client closes connection #1232

* Fix edge case were client responds with invalid header

* Follow linter suggestions for tests

* Changes after review

* Reafactor error check after review

* Handle connection reset on windows

* Remove format string from test where not needed

* Run connection reset tests not on Windows
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

Successfully merging a pull request may close this issue.

2 participants