[tbb-bugs] #17568 [Tor Browser]: Clean up tor-control-port.js in Torbutton

[Tor Bug Tracker & Wiki]

#17568: Clean up tor-control-port.js in Torbutton
-------------------------------------------------+-------------------------
 Reporter:  gk                                   |          Owner:  tbb-
     Type:  task                                 |  team
 Priority:  Medium                               |         Status:
Component:  Tor Browser                          |  needs_review
 Severity:  Normal                               |      Milestone:
 Keywords:  tbb-torbutton,                       |        Version:
  TorBrowserTeam201511R                          |     Resolution:
Parent ID:                                       |  Actual Points:
  Sponsor:                                       |         Points:
-------------------------------------------------+-------------------------

Comment (by arthuredelstein):

 Replying to [comment:7 gk]:
 > Replying to [comment:6 arthuredelstein]:
 > > Replying to [comment:4 gk]:
 > > > Here are some things I thought about while reading the code in tor-
 control-port.js. I am confused about other things(, too) (especially while
 looking at the spec + the controller.c file) but these might be more for a
 second clean-up (after I had more time to sort them out).
 > > >
 > > > 1)
 > > > {{{
 > > >   // GETCONF with a single argument returns results that look like
 > > >   // results from GETINFO with multiple arguments."
 > > > }}}
 > > > Does not seem to be so:
 > > > {{{
 > > > [11-10 15:33:46] Torbutton INFO: controlPort << getconf bridge
 > > >
 > > >
 > > > [11-10 15:33:46] Torbutton INFO: controlPort >> 250 Bridge
 > > > [11-10 15:33:46] Torbutton INFO: controlPort << getinfo version
 config-file
 > > >
 > > >
 > > > [11-10 15:33:46] Torbutton INFO: controlPort >>
 250-version=0.2.7.4-rc (git-f55d23e1e66e9b0f)
 > > > 250-config-file=/path/to/torbrowser/tor-
 browser/Browser/TorBrowser/Data/Tor/torrc
 > > > 250 OK
 > > > }}}
 > >
 > > That's a good point. Right now, we're not parsing getconf lines such
 as `250 Bridge`. We'd need to write some extra code for this. I tried to
 clarify the comment to reflect the current situation.
 >
 > Hmm... Did you really mean `250[+ ]key=value`? It should be something
 like `250[ -]key=value` instead, no? At least that is what I would guess
 reading `handle_control_getconf()` in tor's control.c.

 Oops, dumb mistake. Thanks for catching it.

 > > > 4) I wonder why we only have `getInfoMultiple()` given that both
 GETINFO and GETCONF can have multiple keywords (we use them both with just
 one keyword right now as far as I can see which does not explain the
 different treatment of both to me)
 > >
 > > I never implemented getConfMultiple, but it should be pretty
 straightforward if we need it.
 >
 > Yes, but as I said we don't need `getInfoMultiple()` either at the
 moment as far as I can see. Why do we have this additional code then?

 Sorry I missed the point first time around. You're right, we don't need
 it. I think I wrote it thinking it might be useful in some situation, but
 now I think that's unlikely. I'm removing it for now.

 Here is the new version:
 https://github.com/arthuredelstein/torbutton/commit/17568+1

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