[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

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

#14271: Make Torbutton work with Unix Domain Socket option
-------------------------------------------------+-------------------------
 Reporter:  gk                                   |          Owner:  brade
     Type:  enhancement                          |         Status:
                                                 |  needs_revision
 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 gk):

 * keywords:  tbb-torbutton, tbb-security, TorBrowserTeam201608R => tbb-
     torbutton, tbb-security, TorBrowserTeam201609
 * status:  needs_review => needs_revision


Comment:

 Here are some comments:

 1)
 {{{
 +// given sockFile or host and port.
 }}}
 s/sockFile/socketFile/

 2)
 {{{
 let tlps = Cc["@torproject.org/torlauncher-protocol-service;1"]
              .getService(Ci.nsISupports).wrappedJSObject;
 }}}
 We should do that once and not twice in `torbutton_init()`.

 3) Checking whether we should call `torbutton_local_tor_check()` should
 check for
 `m_tb_control_socket_file` as well (not only for `m_tb_control_port`) I
 guess?

 4) I am not so sure about
 {{{
 +            } catch(e) {
 +              m_tb_control_port = 9151;
 +            }
 }}}
 What was your rationale for adding that now (given you omitted it
 earlier)? For one, the logs might be misleading showing a probably wrong
 port (I mean the setup is seriously troubling if we need to assign `9151`
 in the catch clause at all) in an error with respect to the control
 connection. On the other hand, we might want to show that something is
 wrong with the help of `torbutton_do_tor_check()` which would update the
 toolbar button in this case (if we get that far at all with a broken setup
 like the one in question).

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/14271#comment:12>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs