[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:                       |
-------------------------------------+----------------------------

Comment (by dcf):

 As for adding a test for a failed Close in readAuthCookieFile, I realized
 that doesn't make sense because readAuthCookieFile takes a string
 filename, not an io.ReadCloser. But I still hate to add error-handling
 code that doesn't have a test, or even ever been run.

 One way is to expose an intermediate level of abstraction for the purpose
 of testing:
 {{{
 #!diff
 --- a/pt.go
 +++ b/pt.go
 @@ -546,18 +546,21 @@ func readAuthCookie(f io.Reader) ([]byte, error) {

         return cookie, nil
  }

 +func readAndCloseAuthCookie(f io.ReadCloser) ([]byte, error) {
 +       defer f.Close()
 +       return readAuthCookieFile(f)
 +}
 +
  // Read and validate the contents of an auth cookie file. Returns the
 32-byte
  // cookie. See section 4.2.1.2 of pt-spec.txt.
  func readAuthCookieFile(filename string) ([]byte, error) {
         f, err := os.Open(filename)
         if err != nil {
                 return nil, err
         }
 -       defer f.Close()
 -
 -       return readAuthCookie(f)
 +       return readAndCloseAuthCookie(f)
  }

  // This structure is returned by ServerSetup. It consists of a list of
  // Bindaddrs, an address for the ORPort, an address for the extended
 ORPort (if
 }}}
 Then you can write a test that passes a mock ReadCloser to
 readAndCloseAuthCookie:
 {{{
 // An io.ReadCloser that returns the given error on Close.
 type errorReadCloser struct {
         io.Reader
         err error
 }

 func (rc *errorReadCloser) Close() error {
         return rc.err
 }

 func TestAuthCookieCloseError(t *testing.T) {
         closeErr := errors.New("failed Close")
         // Error reading cookie, also error in Close. The Close error
 should not
         // shadow the read error.
         _, err :=
 readAndCloseAuthCookie(&errorReadCloser{bytes.NewReader([]byte("")),
 closeErr})
         if err == closeErr {
                 t.Errorf("readAndCloseAuthCookie allowed Close error to
 shadow other error")
         }
         // Error in Close after reading cookie.
         _, err =
 readAndCloseAuthCookie(&errorReadCloser{bytes.NewReader([]byte("! Extended
 ORPort Auth Cookie !\x0a0123456789ABCDEF0123456789ABCDEF")), closeErr})
         if err != closeErr {
                 t.Errorf("readAndCloseAuthCookie did not report Close
 error: %v", err)
         }
 }
 }}}

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