[Whonix-devel] #14271 [Applications/Tor Browser]: Make Torbutton work with Unix Domain Socket option
Tor Bug Tracker & Wiki
blackhole at torproject.org
Wed Sep 7 23:16:38 CEST 2016
#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
More information about the Whonix-devel
mailing list