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/http: EOF returned from http.Transport #53472

Open
mtrmac opened this issue Jun 21, 2022 · 5 comments
Open

net/http: EOF returned from http.Transport #53472

mtrmac opened this issue Jun 21, 2022 · 5 comments
Labels
NeedsInvestigation

Comments

@mtrmac
Copy link

@mtrmac mtrmac commented Jun 21, 2022

What version of Go are you using (go version)?

$ go version
go version go1.18.3 darwin/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/mitr/Library/Caches/go-build"
GOENV="/Users/mitr/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/mitr/Go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/mitr/Go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.18.3"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/mitr/Go/src/github.com/containers/image/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/tp/yfcwvlb55vx8lkv5gppb43cm0000gn/T/go-build3737075247=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Given a HTTP server that reads the request, but (cleanly) closes the connection without producing any response:

package main

import (
	"fmt"
	"log"
	"net"
	"net/http"
)

func server(ln net.Listener) {
	for {
		conn, err := ln.Accept()
		if err != nil {
			return
		}
		log.Printf("%v: Accepted", conn.RemoteAddr())
		var buf [4096]byte          // Hopefully enough for a full header
		n, err := conn.Read(buf[:]) // Completely read and ignore the header
		if err != nil {
			panic(err)
		}
		log.Printf("%v: Read %d", conn.RemoteAddr(), n)
		err = conn.Close()
		if err != nil {
			panic(err)
		}
		log.Printf("%v: Closed", conn.RemoteAddr())
	}
}

func main() {
	ln, err := net.Listen("tcp", "127.0.0.1:0")
	if err != nil {
		panic(err)
	}
	go server(ln)

	res, err := http.Get(fmt.Sprintf("http://%s/", ln.Addr().String()))
	fmt.Printf("res=%#v, err=%v (%#v)", res, err, err)
}

What did you expect to see?

An error saying something about an unexpectedly closed connection.

What did you see instead?

2022/06/21 02:28:30 127.0.0.1:64135: Accepted
2022/06/21 02:28:30 127.0.0.1:64135: Read 96
2022/06/21 02:28:30 127.0.0.1:64135: Closed
res=(*http.Response)(nil), err=Get "http://127.0.0.1:64134/": EOF (&url.Error{Op:"Get", URL:"http://127.0.0.1:64134/", Err:(*errors.errorString)(0xc000098060)})

i.e. the error is io.EOF, which seems inconsistent with the official definition of that value:

Functions should return EOF only to signal a graceful end of input. If the EOF occurs unexpectedly in a structured data stream, the appropriate error is either ErrUnexpectedEOF or some other error giving more detail.

Notes

I appreciate that this might not be possible to change due to the compatibility promise.

The immediate cause is

_, err := pc.br.Peek(1)
; if that returns io.EOF, it is wrapped in
err = transportReadFromServerError{err}
, but later only unwrapped to become raw io.EOF again, with no logic anywhere to turn it into an “this was unexpected” error.

@seankhliao
Copy link
Contributor

@seankhliao seankhliao commented Jun 21, 2022

that's not a raw EOF, it's a net/url.Error

@mtrmac
Copy link
Author

@mtrmac mtrmac commented Jun 21, 2022

Oops, you’re completely correct. I was too focused on the user-visible text rather than on the type.

@mtrmac
Copy link
Author

@mtrmac mtrmac commented Jun 21, 2022

At least using the http.Transport implementation directly (keeping the same server method) really returns an io.EOF:

func main() {
	ln, err := net.Listen("tcp", "127.0.0.1:0")
	if err != nil {
		panic(err)
	}
	go server(ln)

	req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("http://%s/", ln.Addr().String()), nil)
	if err != nil {
		panic(err)
	}
	res, err := http.DefaultTransport.RoundTrip(req)
	fmt.Printf("res=%#v, err=%v (%#v) isEOF=%v\n", res, err, err, err == io.EOF)
}

prints

res=(*http.Response)(nil), err=EOF (&errors.errorString{s:"EOF"}) isEOF=true

@mtrmac
Copy link
Author

@mtrmac mtrmac commented Jun 21, 2022

To be specific, my primary concern is about the lack of a clear user-visible error message text. Right now error messages in logs just say “EOF” with little to allow diagnosing the failure further: it doesn’t indicate the relevant component (client / server / proxy / hypothetically something else like a corrupt local (certificate?) file).

I can, of course, wrap the http.Do call with a check for net.url.Error{Err: io.EOF}, and replace the text, but … what’s a long-term reasonable replacement text? Hard-coding a knowledge about this particular code path might not be accurate if such an "EOF" string could possibly be returned by any other error path, and it is brittle WRT any future changes of the net/http implementation.

So I’d prefer receiving a more descriptive error message from the net/http client.

@seankhliao seankhliao changed the title net/http: EOF returned from http.Client.Do net/http: EOF returned from http.Transport Jun 22, 2022
@seankhliao
Copy link
Contributor

@seankhliao seankhliao commented Jun 22, 2022

cc @neild

@seankhliao seankhliao added the NeedsInvestigation label Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation
Projects
None yet
Development

No branches or pull requests

2 participants