[tor-bugs] #14271 [Applications/Tor Browser]: Make Torbutton work with Unix Domain Socket option

[Tor Bug Tracker & Wiki]

#14271: Make Torbutton work with Unix Domain Socket option
-------------------------------------------------+-------------------------
 Reporter:  gk                                   |          Owner:  brade
     Type:  enhancement                          |         Status:
                                                 |  needs_information
 Priority:  High                                 |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tbb-torbutton, tbb-security,         |  Actual Points:
  TorBrowserTeam201609                           |
Parent ID:  #14270                               |         Points:
 Reviewer:                                       |        Sponsor:
                                                 |  SponsorU
-------------------------------------------------+-------------------------
Changes (by mcs):

 * keywords:  tbb-torbutton, tbb-security, TorBrowserTeam201609R => tbb-
     torbutton, tbb-security, TorBrowserTeam201609
 * status:  needs_review => needs_information


Comment:

 Replying to [comment:16 arthuredelstein]:
 > Sorry for the late review. This patch looks good to me -- nice work! I
 have a couple of minor stylistic suggestions that you may or may not want
 to adopt:
 > {{{
 > +// given socketFile or host and port.
 > +io.asyncSocketStreams = function (socketFile, host, port) {
 > +  let socketTransport;
 > +  let sts = Cc["@mozilla.org/network/socket-transport-service;1"]
 > +
 .getService(Components.interfaces.nsISocketTransportService),
 > +       UNBUFFERED = Ci.nsITransport.OPEN_UNBUFFERED;
 > +
 > +  // Create an instance of a socket transport.
 > +  if (socketFile) {
 > +    socketTransport = sts.createUnixDomainTransport(socketFile);
 > +  } else {
 > +    socketTransport = sts.createTransport(null, 0, host, port, null);
 > +  }
 > +
 > }}}
 > Maybe move `let socketTransport` down to after the comment `// Create an
 instance of a socket transport`?

 Good idea.

 > Also, in that file, I had used blank lines to separate functions, though
 that's probably a personal eccentricity...

 I am OK with accommodating eccentricities, but can you explain what we
 need to fix? I don't think we removed any blank lines, but maybe I am just
 not seeing it.

 >
 > Also, a somewhat bigger suggestion that could be part of this patch or
 left for another time. In `torbutton.js`, we will now have
 > {{{
 >  var m_tb_control_socket_file = null; // Set if using a UNIX domain
 socket.
 >  var m_tb_control_port = null;        // Set if not using a socket.
 >  var m_tb_control_host = null;        // Set if not using a socket.
 >  var m_tb_control_pass = null;
 >  var m_tb_control_desc = null;
 > }}}
 > I imagine it might be cleaner to collect these into a single data object
 like
 > {{{
 > var m_tb_control = { socket_file, host, port, password, descriptor }
 > }}}
 > Then we could factor out a single factory function from
 `torbutton_init()` that generates this object. And we could use this
 object as a single argument for functions in `tor-control-port.js` and
 `tor-circuit-display.js` as well.

 Kathy and I like that suggestion, but let's do it as a follow up. I
 created #20102 for it.

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