[tor-bugs] #3302 [Pluggable transport]: obfs2 delays sending pending data

[Tor Bug Tracker & Wiki]

#3302: obfs2 delays sending pending data
---------------------------------+------------------------------------------
 Reporter:  asn                  |          Owner:  asn
     Type:  defect               |         Status:  new
 Priority:  normal               |      Milestone:     
Component:  Pluggable transport  |        Version:     
 Keywords:                       |         Parent:     
   Points:                       |   Actualpoints:     
---------------------------------+------------------------------------------

Comment(by nickm):

 Replying to [comment:5 asn]:
 > Replying to [comment:4 nickm]:
 > > I really prefer the solution I outlined where obfs2_recv() can return
 something that indicates that the caller needs to do an obfs2_send() right
 away without waiting any data.
 > >
 >
 > Yes, I like that solution myself, but the implementation seems dirtier
 than it sounds on words. Basically, the caller of obfs2_send() is
 proto_send(). proto_send() can do the same things that obfs2_send() can
 do, so we have to go back to plaintext_read_cb(). We can indeed call
 obfs2_send() correctly from there, but isn't it ugly?
 > I know I'm stuck on the concept of abstraction when we just have one
 protocol, but still defining a protocol specific return code (since
 calling proto_send() right away shouldn't make sense on other protocols)
 on network.c gives me twitches.

 I like abstraction too: so don't make it a protocol-specific return code.
 Instead, it could be a generic "You should call proto_send()" thing rather
 than a protocol-specific "You should call obfs2_send()" thing.

 > > Having the listeners have a list of connections just seems wrong, if
 the whole point is to serve as a socks_state-to-connection map.  If the
 protocol send and recv methods really truly need access to the conn_t,
 then let's just pass them the conn_t!  The functions that call proto_send
 and proto_recv have the conn_t already: no need to make the functions they
 call hunt for it.
 >
 > You are right, doing all that just to get a socks_state-to-connection
 map is stupid. I just found it intuitive in the sense that the point of a
 listener is to accept connections and that would give us a way to go from
 a listener to a connection.
 >
 > Passing `conn_t` to the protocols is ugly as well, but it provides us
 with ways to solve other similar problems (like #3291).

 One thing I dislike about the passing conn_t solution is that it makes the
 protocols harder to test: it is much easier to make sure that they do the
 right things with evbuffers than it is to make sure that they do the right
 things with bufferevents.

 Alternatively, we _could_ just make the protocol state contain a pointer
 to the evbuffer that we want to flush pending data into when the handshake
 completes.  That's a hack too, but at least it isolates everything into
 the protocol layer.

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