commit 638ec6c222327ca02950338b2919ce8b22e1f900
Author: Cecylia Bocovich <coh...@torproject.org>
Date:   Thu Sep 9 16:01:38 2021 -0400

    Update Snowflake client library documentation
    
    Follow best practices for documenting the exported pieces of the
    Snowflake client library.
---
 client/lib/interfaces.go          | 17 +++++++-------
 client/lib/peers.go               | 18 +++++++-------
 client/lib/rendezvous.go          | 23 +++++++-----------
 client/lib/rendezvous_ampcache.go |  4 ++--
 client/lib/rendezvous_http.go     |  2 +-
 client/lib/rendezvous_test.go     |  4 ++--
 client/lib/snowflake.go           | 49 ++++++++++++++++++++++++++++++++-------
 client/lib/webrtc.go              | 16 ++++++++-----
 8 files changed, 83 insertions(+), 50 deletions(-)

diff --git a/client/lib/interfaces.go b/client/lib/interfaces.go
index 0a871b8..66b9d56 100644
--- a/client/lib/interfaces.go
+++ b/client/lib/interfaces.go
@@ -1,23 +1,24 @@
 package lib
 
-// Interface for catching Snowflakes. (aka the remote dialer)
+// Tongue is an interface for catching Snowflakes. (aka the remote dialer)
 type Tongue interface {
+       // Catch makes a connection to a new snowflake.
        Catch() (*WebRTCPeer, error)
 
-       // Get the maximum number of snowflakes
+       // GetMax returns the maximum number of snowflakes a client can have.
        GetMax() int
 }
 
-// Interface for collecting some number of Snowflakes, for passing along
-// ultimately to the SOCKS handler.
+// SnowflakeCollector is an interface for managing a client's collection of 
snowflakes.
 type SnowflakeCollector interface {
-       // Add a Snowflake to the collection.
-       // Implementation should decide how to connect and maintain the 
webRTCConn.
+       // Collect adds a snowflake to the collection.
+       // The implementation of Collect should decide how to connect to and 
maintain
+       // the connection to the WebRTCPeer.
        Collect() (*WebRTCPeer, error)
 
-       // Remove and return the most available Snowflake from the collection.
+       // Pop removes and returns the most available snowflake from the 
collection.
        Pop() *WebRTCPeer
 
-       // Signal when the collector has stopped collecting.
+       // Melted returns a channel that will signal when the collector has 
stopped.
        Melted() <-chan struct{}
 }
diff --git a/client/lib/peers.go b/client/lib/peers.go
index 1abcd95..6bddbf2 100644
--- a/client/lib/peers.go
+++ b/client/lib/peers.go
@@ -8,7 +8,7 @@ import (
        "sync"
 )
 
-// Container which keeps track of multiple WebRTC remote peers.
+// Peers is a container that keeps track of multiple WebRTC remote peers.
 // Implements |SnowflakeCollector|.
 //
 // Maintaining a set of pre-connected Peers with fresh but inactive 
datachannels
@@ -31,7 +31,7 @@ type Peers struct {
        collectLock sync.Mutex
 }
 
-// Construct a fresh container of remote peers.
+// NewPeers constructs a fresh container of remote peers.
 func NewPeers(tongue Tongue) (*Peers, error) {
        p := &Peers{}
        // Use buffered go channel to pass snowflakes onwards to the SOCKS 
handler.
@@ -45,7 +45,7 @@ func NewPeers(tongue Tongue) (*Peers, error) {
        return p, nil
 }
 
-// As part of |SnowflakeCollector| interface.
+// Collect connects to and adds a new remote peer as part of 
|SnowflakeCollector| interface.
 func (p *Peers) Collect() (*WebRTCPeer, error) {
        // Engage the Snowflake Catching interface, which must be available.
        p.collectLock.Lock()
@@ -76,8 +76,8 @@ func (p *Peers) Collect() (*WebRTCPeer, error) {
        return connection, nil
 }
 
-// Pop blocks until an available, valid snowflake appears. Returns nil after 
End
-// has been called.
+// Pop blocks until an available, valid snowflake appears.
+// Pop will return nil after End has been called.
 func (p *Peers) Pop() *WebRTCPeer {
        for {
                snowflake, ok := <-p.snowflakeChan
@@ -93,12 +93,13 @@ func (p *Peers) Pop() *WebRTCPeer {
        }
 }
 
-// As part of |SnowflakeCollector| interface.
+// Melted returns a channel that will close when peers stop being collected.
+// Melted is a necessary part of |SnowflakeCollector| interface.
 func (p *Peers) Melted() <-chan struct{} {
        return p.melt
 }
 
-// Returns total available Snowflakes (including the active one)
+// Count returns the total available Snowflakes (including the active ones)
 // The count only reduces when connections themselves close, rather than when
 // they are popped.
 func (p *Peers) Count() int {
@@ -118,7 +119,8 @@ func (p *Peers) purgeClosedPeers() {
        }
 }
 
-// Close all Peers contained here.
+// End closes all active connections to Peers contained here, and stops the
+// collection of future Peers.
 func (p *Peers) End() {
        close(p.melt)
        p.collectLock.Lock()
diff --git a/client/lib/rendezvous.go b/client/lib/rendezvous.go
index cf67e09..689e51c 100644
--- a/client/lib/rendezvous.go
+++ b/client/lib/rendezvous.go
@@ -1,10 +1,5 @@
 // WebRTC rendezvous requires the exchange of SessionDescriptions between
 // peers in order to establish a PeerConnection.
-//
-// This file contains the one method currently available to Snowflake:
-//
-// - Domain-fronted HTTP signaling. The Broker automatically exchange offers
-//   and answers between this client and some remote WebRTC proxy.
 
 package lib
 
@@ -22,7 +17,7 @@ import (
 )
 
 const (
-       BrokerErrorUnexpected string = "Unexpected error, no answer."
+       brokerErrorUnexpected string = "Unexpected error, no answer."
        readLimit                    = 100000 //Maximum number of bytes to be 
read from an HTTP response
 )
 
@@ -55,7 +50,7 @@ func createBrokerTransport() http.RoundTripper {
        return transport
 }
 
-// Construct a new BrokerChannel, where:
+// NewBrokerChannel construct a new BrokerChannel, where:
 // |broker| is the full URL of the facilitating program which assigns proxies
 // to clients, and |front| is the option fronting domain.
 func NewBrokerChannel(broker, ampCache, front string, keepLocalAddresses bool) 
(*BrokerChannel, error) {
@@ -85,10 +80,8 @@ func NewBrokerChannel(broker, ampCache, front string, 
keepLocalAddresses bool) (
        }, nil
 }
 
-// Roundtrip HTTP POST using WebRTC SessionDescriptions.
-//
-// Send an SDP offer to the broker, which assigns a proxy and responds
-// with an SDP answer from a designated remote WebRTC peer.
+// Negotiate uses a RendezvousMethod to send the client's WebRTC SDP offer
+// and receive a snowflake proxy WebRTC SDP answer in return.
 func (bc *BrokerChannel) Negotiate(offer *webrtc.SessionDescription) (
        *webrtc.SessionDescription, error) {
        // Ideally, we could specify an `RTCIceTransportPolicy` that would 
handle
@@ -135,6 +128,7 @@ func (bc *BrokerChannel) Negotiate(offer 
*webrtc.SessionDescription) (
        return util.DeserializeSessionDescription(resp.Answer)
 }
 
+// SetNATType sets the NAT type of the client so we can send it to the WebRTC 
broker.
 func (bc *BrokerChannel) SetNATType(NATType string) {
        bc.lock.Lock()
        bc.natType = NATType
@@ -142,13 +136,14 @@ func (bc *BrokerChannel) SetNATType(NATType string) {
        log.Printf("NAT Type: %s", NATType)
 }
 
-// Implements the |Tongue| interface to catch snowflakes, using BrokerChannel.
+// WebRTCDialer implements the |Tongue| interface to catch snowflakes, using 
BrokerChannel.
 type WebRTCDialer struct {
        *BrokerChannel
        webrtcConfig *webrtc.Configuration
        max          int
 }
 
+// NewWebRTCDialer constructs a new WebRTCDialer.
 func NewWebRTCDialer(broker *BrokerChannel, iceServers []webrtc.ICEServer, max 
int) *WebRTCDialer {
        config := webrtc.Configuration{
                ICEServers: iceServers,
@@ -161,14 +156,14 @@ func NewWebRTCDialer(broker *BrokerChannel, iceServers 
[]webrtc.ICEServer, max i
        }
 }
 
-// Initialize a WebRTC Connection by signaling through the broker.
+// Catch initializes a WebRTC Connection by signaling through the 
BrokerChannel.
 func (w WebRTCDialer) Catch() (*WebRTCPeer, error) {
        // TODO: [#25591] Fetch ICE server information from Broker.
        // TODO: [#25596] Consider TURN servers here too.
        return NewWebRTCPeer(w.webrtcConfig, w.BrokerChannel)
 }
 
-// Returns the maximum number of snowflakes to collect
+// GetMax returns the maximum number of snowflakes to collect.
 func (w WebRTCDialer) GetMax() int {
        return w.max
 }
diff --git a/client/lib/rendezvous_ampcache.go 
b/client/lib/rendezvous_ampcache.go
index 2f1fb9f..6ac99b3 100644
--- a/client/lib/rendezvous_ampcache.go
+++ b/client/lib/rendezvous_ampcache.go
@@ -93,7 +93,7 @@ func (r *ampCacheRendezvous) Exchange(encPollReq []byte) 
([]byte, error) {
                // * If the broker returns a 5xx status, the AMP cache
                //   translates it to a 404.
                // 
https://amp.dev/documentation/guides-and-tutorials/learn/amp-caches-and-cors/amp-cache-urls/#redirect-%26-error-handling
-               return nil, errors.New(BrokerErrorUnexpected)
+               return nil, errors.New(brokerErrorUnexpected)
        }
        if _, err := resp.Location(); err == nil {
                // The Google AMP Cache may return a "silent redirect" with
@@ -103,7 +103,7 @@ func (r *ampCacheRendezvous) Exchange(encPollReq []byte) 
([]byte, error) {
                // follow redirects nor execute JavaScript, but in any case we
                // cannot extract information from this response and can only
                // treat it as an error.
-               return nil, errors.New(BrokerErrorUnexpected)
+               return nil, errors.New(brokerErrorUnexpected)
        }
 
        lr := io.LimitReader(resp.Body, readLimit+1)
diff --git a/client/lib/rendezvous_http.go b/client/lib/rendezvous_http.go
index e020077..43ed075 100644
--- a/client/lib/rendezvous_http.go
+++ b/client/lib/rendezvous_http.go
@@ -60,7 +60,7 @@ func (r *httpRendezvous) Exchange(encPollReq []byte) ([]byte, 
error) {
 
        log.Printf("HTTP rendezvous response: %s", resp.Status)
        if resp.StatusCode != http.StatusOK {
-               return nil, errors.New(BrokerErrorUnexpected)
+               return nil, errors.New(brokerErrorUnexpected)
        }
 
        return limitedRead(resp.Body, readLimit)
diff --git a/client/lib/rendezvous_test.go b/client/lib/rendezvous_test.go
index 6a1a071..4bc5766 100644
--- a/client/lib/rendezvous_test.go
+++ b/client/lib/rendezvous_test.go
@@ -122,7 +122,7 @@ func TestHTTPRendezvous(t *testing.T) {
                        answer, err := rend.Exchange(fakeEncPollReq)
                        So(err, ShouldNotBeNil)
                        So(answer, ShouldBeNil)
-                       So(err.Error(), ShouldResemble, BrokerErrorUnexpected)
+                       So(err.Error(), ShouldResemble, brokerErrorUnexpected)
                })
 
                Convey("httpRendezvous.Exchange fails with error", func() {
@@ -243,7 +243,7 @@ func TestAMPCacheRendezvous(t *testing.T) {
                        answer, err := rend.Exchange(fakeEncPollReq)
                        So(err, ShouldNotBeNil)
                        So(answer, ShouldBeNil)
-                       So(err.Error(), ShouldResemble, BrokerErrorUnexpected)
+                       So(err.Error(), ShouldResemble, brokerErrorUnexpected)
                })
 
                Convey("ampCacheRendezvous.Exchange fails with error", func() {
diff --git a/client/lib/snowflake.go b/client/lib/snowflake.go
index e0591a7..4b17f0b 100644
--- a/client/lib/snowflake.go
+++ b/client/lib/snowflake.go
@@ -17,12 +17,21 @@ import (
 )
 
 const (
+       // ReconnectTimeout is the time a Snowflake client will wait before 
collecting
+       // more snowflakes.
        ReconnectTimeout = 10 * time.Second
+       // SnowflakeTimeout is the time a Snowflake client will wait before 
determining that
+       // a remote snowflake has been disconnected. If no new messages are 
sent or received
+       // in this time period, the client will terminate the connection with 
the remote
+       // peer and collect a new snowflake.
        SnowflakeTimeout = 20 * time.Second
-       // How long to wait for the OnOpen callback on a DataChannel.
+       // DataChannelTimeout is how long the client will wait for the OnOpen 
callback
+       // on a newly created DataChannel.
        DataChannelTimeout = 10 * time.Second
 
+       // WindowSize is the number of packets in the send and receive window 
of a KCP connection.
        WindowSize = 65535
+       // StreamSize controls the maximum amount of in flight data between a 
client and server.
        StreamSize = 1048576 //1MB
 )
 
@@ -37,16 +46,31 @@ type Transport struct {
        dialer *WebRTCDialer
 }
 
+// ClientConfig defines how the SnowflakeClient will connect to the broker and 
Snowflake proxies.
 type ClientConfig struct {
-       BrokerURL          string
-       AmpCacheURL        string
-       FrontDomain        string
-       ICEAddresses       []string
+       // BrokerURL is the full URL of the Snowflake broker that the client 
will connect to.
+       BrokerURL string
+       // AmpCacheURL is the full URL of a valid AMP cache. A nonzero value 
indicates
+       // that AMP cache will be used as the rendezvous method with the broker.
+       AmpCacheURL string
+       // FrontDomain is a the full URL of an optional front domain that can 
be used with either
+       // the AMP cache or HTTP domain fronting rendezvous method.
+       FrontDomain string
+       // ICEAddresses are a slice of ICE server URLs that will be used for 
NAT traversal and
+       // the creation of the client's WebRTC SDP offer.
+       ICEAddresses []string
+       // KeepLocalAddresses is an optional setting that will prevent the 
removal of local or
+       // invalid addresses from the client's SDP offer. This is useful for 
local deployments
+       // and testing.
        KeepLocalAddresses bool
-       Max                int
+       // Max is the maximum number of snowflake proxy peers that the client 
should attempt to
+       // connect to.
+       Max int
 }
 
-// Create a new Snowflake transport client that can spawn multiple Snowflake 
connections.
+// NewSnowflakeClient creates a new Snowflake transport client that can spawn 
multiple
+// Snowflake connections.
+//
 // brokerURL and frontDomain are the urls for the broker host and domain 
fronting host
 // iceAddresses are the STUN/TURN urls needed for WebRTC negotiation
 // keepLocalAddresses is a flag to enable sending local network addresses (for 
testing purposes)
@@ -82,8 +106,10 @@ func NewSnowflakeClient(config ClientConfig) (*Transport, 
error) {
        return transport, nil
 }
 
-// Create a new Snowflake connection. Starts the collection of snowflakes and 
returns a
-// smux Stream.
+// Dial creates a new Snowflake connection.
+// Dial starts the collection of snowflakes and returns a SnowflakeConn that 
is a
+// wrapper around a smux.Stream that will reliably deliver data to a Snowflake
+// server through one or more snowflake proxies.
 func (t *Transport) Dial() (net.Conn, error) {
        // Cleanup functions to run before returning, in case of an error.
        var cleanup []func()
@@ -132,10 +158,12 @@ func (t *Transport) Dial() (net.Conn, error) {
        return &SnowflakeConn{Stream: stream, sess: sess, pconn: pconn, 
snowflakes: snowflakes}, nil
 }
 
+// SetRendezvousMethod sets the rendezvous method to the Snowflake broker.
 func (t *Transport) SetRendezvousMethod(r RendezvousMethod) {
        t.dialer.Rendezvous = r
 }
 
+// SnowflakeConn is a reliable connection to a snowflake server that 
implements net.Conn.
 type SnowflakeConn struct {
        *smux.Stream
        sess       *smux.Session
@@ -143,6 +171,9 @@ type SnowflakeConn struct {
        snowflakes *Peers
 }
 
+// Close closes the connection.
+//
+// The collection of snowflake proxies for this connection is stopped.
 func (conn *SnowflakeConn) Close() error {
        log.Printf("---- SnowflakeConn: closed stream %v ---", conn.ID())
        conn.Stream.Close()
diff --git a/client/lib/webrtc.go b/client/lib/webrtc.go
index 6fc24c0..538cc8b 100644
--- a/client/lib/webrtc.go
+++ b/client/lib/webrtc.go
@@ -12,10 +12,9 @@ import (
        "github.com/pion/webrtc/v3"
 )
 
-// Remote WebRTC peer.
+// WebRTCPeer represents a WebRTC connection to a remote snowflake proxy.
 //
-// Handles preparation of go-webrtc PeerConnection. Only ever has
-// one DataChannel.
+// Each WebRTCPeer only ever has one DataChannel that is used as the peer's 
transport.
 type WebRTCPeer struct {
        id        string
        pc        *webrtc.PeerConnection
@@ -35,7 +34,11 @@ type WebRTCPeer struct {
        bytesLogger bytesLogger
 }
 
-// Construct a WebRTC PeerConnection.
+// NewWebRTCPeer constructs a WebRTC PeerConnection to a snowflake proxy.
+//
+// The creation of the peer handles the signaling to the Snowflake broker, 
including
+// the exchange of SDP information, the creation of a PeerConnection, and the 
establishment
+// of a DataChannel to the Snowflake proxy.
 func NewWebRTCPeer(config *webrtc.Configuration,
        broker *BrokerChannel) (*WebRTCPeer, error) {
        connection := new(WebRTCPeer)
@@ -79,7 +82,7 @@ func (c *WebRTCPeer) Write(b []byte) (int, error) {
        return len(b), nil
 }
 
-//Returns a boolean indicated whether the peer is closed
+// Closed returns a boolean indicated whether the peer is closed.
 func (c *WebRTCPeer) Closed() bool {
        select {
        case <-c.closed:
@@ -89,6 +92,7 @@ func (c *WebRTCPeer) Closed() bool {
        return false
 }
 
+// Close closes the connection the snowflake proxy.
 func (c *WebRTCPeer) Close() error {
        c.once.Do(func() {
                close(c.closed)
@@ -225,7 +229,7 @@ func (c *WebRTCPeer) preparePeerConnection(config 
*webrtc.Configuration) error {
        return nil
 }
 
-// Close all channels and transports
+// cleanup closes all channels and transports
 func (c *WebRTCPeer) cleanup() {
        // Close this side of the SOCKS pipe.
        if c.writePipe != nil { // c.writePipe can be nil in tests.



_______________________________________________
tor-commits mailing list
tor-commits@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits

Reply via email to