nemith / netconf

NETCONF implementation in Go.
Other
29 stars 7 forks source link

BUG in main receive loop - it does not finish after TCP connection was closed #84

Open ksproska opened 6 days ago

ksproska commented 6 days ago

Implementation

In recv() method that is started as a separate goroutine the loop is supposed to stop when io.EOF or io.ErrUnexpectedEOF error is received, however even though connection was successfully closed (for tcp connection) that is not a case and error "use of closed network connection" is printed the whole way through the run of the program suggesting therefore that the recv() method never finishes it's run.

Recognizing connection termination as implemented in apimachinery library

In apimachinery/pkg/util/net/http.go function IsProbableEOF was implemented that besides recognizing connection termination by receiving error io.EOF or io.ErrUnexpectedEOF analysis also other errors that "resemble a connection termination". The function in question:

func IsProbableEOF(err error) bool {
    if err == nil {
        return false
    }
    var uerr *url.Error
    if errors.As(err, &uerr) {
        err = uerr.Err
    }
    msg := err.Error()
    switch {
    case err == io.EOF:
        return true
    case err == io.ErrUnexpectedEOF:
        return true
    case msg == "http: can't write HTTP request on broken connection":
        return true
    case strings.Contains(msg, "http2: server sent GOAWAY and closed the connection"):
        return true
    case strings.Contains(msg, "connection reset by peer"):
        return true
    case strings.Contains(strings.ToLower(msg), "use of closed network connection"):
        return true
    }
    return false
}

Suggested fix

To fix the described bug I would suggest adding either one new condition for "use of closed network connection" error (which is the bug that I found) or to implement the whole function with all listed error messages so that recv method resembles this:

func (s *Session) recv() {
    var err error
    for {
        err = s.recvMsg()
        if IsProbableEOF(err) {
            break
        }
        if err != nil {
            log.Printf("netconf: failed to read incoming message: %v", err)
        }
    }
    // the rest of the method
}