[tor-bugs] #33144 [Circumvention/Snowflake]: Overhaul websocketconn.Conn; implement net.Conn

[Tor Bug Tracker & Wiki]

#33144: Overhaul websocketconn.Conn; implement net.Conn
-------------------------------------+------------------------------
 Reporter:  dcf                      |          Owner:  dcf
     Type:  enhancement              |         Status:  needs_review
 Priority:  Medium                   |      Milestone:
Component:  Circumvention/Snowflake  |        Version:
 Severity:  Normal                   |     Resolution:
 Keywords:                           |  Actual Points:
Parent ID:                           |         Points:
 Reviewer:                           |        Sponsor:
-------------------------------------+------------------------------
Description changed by dcf:

Old description:

> In the turbotunnel branch I found it convenient to have
> `websocketconn.Conn` implement `net.Conn`, not only `io.ReadWriteCloser`.
> While implementing the additional methods, I found some ways that
> `websocketconn.Conn`'s existing methods do not satisfy the requirements
> of `net.Conn`:
>  * there are data races when multiple goroutines call `Read` or `Write`.
>    * this is partly because `Read` assigns to the shared field `conn.r`,
> and partly because [https://godoc.org/github.com/gorilla/websocket#hdr-
> Concurrency the underlying websocket.Conn doesn't permit concurrent
> access].
>  * when a websocketconn.Conn is closed, it starts returning some kind of
> [https://godoc.org/github.com/gorilla/websocket#CloseError
> websocket.CloseError], not `io.EOF`.
>
> This branch
>  1. adds tests that expose the above issues
>  2. rewrites `websocketconn.Conn` to serialize `Read`s and `Write`s using
> my favorite `io.Pipe`-with-goroutine trick
>  3. implements the remaining `net.Conn` methods

New description:

 In the turbotunnel branch I found it convenient to have
 `websocketconn.Conn` implement `net.Conn`, not only `io.ReadWriteCloser`.
 While implementing the additional methods, I found some ways that
 `websocketconn.Conn`'s existing methods do not satisfy the requirements of
 `net.Conn`:
  * there are data races when multiple goroutines call `Read` or `Write`.
    * this is partly because `Read` assigns to the shared field `conn.r`,
 and partly because [https://godoc.org/github.com/gorilla/websocket#hdr-
 Concurrency the underlying websocket.Conn doesn't permit concurrent
 access].
  * when a websocketconn.Conn is closed, it starts returning some kind of
 [https://godoc.org/github.com/gorilla/websocket#CloseError
 websocket.CloseError], not `io.EOF`.

 This branch
  1. adds tests that expose the above issues
  2. rewrites `websocketconn.Conn` to serialize `Read`s and `Write`s using
 my favorite `io.Pipe`-with-goroutine trick
  3. transforms [https://godoc.org/github.com/gorilla/websocket#CloseError
 websocket.CloseError] with code `CloseNormalClosure` or
 `CloseNoStatusReceived` to `io.EOF`
  4. implements the remaining `net.Conn` methods

--

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