[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #17568 [Tor Browser]: Clean up tor-control-port.js in Torbutton
#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
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs