[tor-bugs] #15826 [Pluggable transport]: Check and return error values in goptlib

[Tor Bug Tracker & Wiki]

#15826: Check and return error values in goptlib
-------------------------------------+----------------------------
     Reporter:  gsathya              |      Owner:  asn
         Type:  defect               |     Status:  needs_revision
     Priority:  normal               |  Milestone:
    Component:  Pluggable transport  |    Version:
   Resolution:                       |   Keywords:  goptlib
Actual Points:                       |  Parent ID:
       Points:                       |
-------------------------------------+----------------------------
Changes (by dcf):

 * status:  new => needs_revision
 * keywords:   => goptlib


Comment:

 > git.torproject.org/pluggable-transports/goptlib.git/pt.go:629:16
 io.WriteString(h, "ExtORPort authentication server-to-client hash")
 > git.torproject.org/pluggable-transports/goptlib.git/pt.go:630:9
 h.Write(clientNonce)
 > git.torproject.org/pluggable-transports/goptlib.git/pt.go:631:9
 h.Write(serverNonce)
 > git.torproject.org/pluggable-transports/goptlib.git/pt.go:638:16
 io.WriteString(h, "ExtORPort authentication client-to-server hash")
 > git.torproject.org/pluggable-transports/goptlib.git/pt.go:639:9
 h.Write(clientNonce)
 > git.torproject.org/pluggable-transports/goptlib.git/pt.go:640:9
 h.Write(serverNonce)

 These are fine and deliberate. [https://golang.org/pkg/hash/#Hash
 Hash.Write "never returns an error."]

 > git.torproject.org/pluggable-transports/goptlib.git/pt.go:857:15
 s.SetDeadline(time.Now().Add(5 * time.Second))
 > git.torproject.org/pluggable-transports/goptlib.git/pt.go:868:15
 s.SetDeadline(time.Time{})

 These ones look good.

 > git.torproject.org/pluggable-transports/goptlib.git/pt.go:557:15
 defer f.Close()

 I think I would like this better as
 {{{
         defer func() {
                 ferr := f.Close()
                 if err == nil {
                         err = ferr
                 }
         }()
 }}}
 Please add tests with a mock Closer that fails. Add test cases for:
  * readAuthCookie succeeds, Close fails.
  * readAuthCookie fails, Close fails (e.g., readAuthCookie gets a
 bytes.Reader that is too short; Close error should not overwrite
 readAuthCookie error)

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/15826#comment:1>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online