Skip to content

The error returned by Stream.Read and .Write on a deadline error violates the net.Conn contract. #156

@espadolini

Description

@espadolini

The net.Conn.SetDeadline documentation (from go 1.25) states:

If the deadline is exceeded a call to Read or Write or to other I/O methods will return an error that wraps os.ErrDeadlineExceeded. This can be tested using errors.Is(err, os.ErrDeadlineExceeded).

Quite a lot of things in the standard library would not actually behave correctly when encountering an error that wraps os.ErrDeadlineExceeded (like net/http.Server, as fixed in #91) because they end up type asserting on net.Error or by checking for methods implemented by the error without any unwrapping, but the one that threw me for a loop and prompted this issue is the fact that crypto/tls.Conn will actually choke on a deadline error returned by Read if it doesn't implement net.Error or if the (deprecated!!!) Temporary() method returns false - and that's currently the case for ErrTimeout, unfortunately.

The end result of this issue is that running a http.Server on a tls.Conn connection built on top of a Stream will fail to upgrade to a websocket because the connection hijack will end up closing the connection.

The simplest fix might be to ignore the os.ErrDeadlineExceeded bit and just return true from Temporary() in ErrTimeout - perhaps while also unwrapping into os.ErrDeadlineExceeded? I don't know if getting rid of NetError and using os.ErrDeadlineExceeded would be an unacceptable backwards compatibility break, but the sad state of affairs is that whatever error is returned by Stream.Read on a deadline must directly implement net.Error and return true from both Timeout() (for http.Server) and Temporary() (for tls.Conn) for read deadlines to behave nondestructively, so only wrapping os.ErrDeadlineExceeded without also changing Temporary() would technically fulfill the net.Conn spec but still misbehave with stdlib constructions.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions