[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 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.
> > 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?
> Replying to [comment:5 gk]:
> > {{{
> > // matchResult finds a single-line result for `250-` or a multi-line
one for `250+`.
> > }}}
> > is interesting as you are not mentioning the `250 ` case although your
regex is checking for it. But I think the comment is right here (but not
the code) as the `250 ` case should not make it that far as I guess
`EndReplyLine` does not contain a `=` (see the `250 Bridge` example) and
`key` would therefore be `null`. That's one of the things I still need to
figure out which is a bit tricky as `ReplyText` is not specified in the
spec.
>
> If you activate a bridge in Tor Browser, such as obfs3, then you get:
>
> {{{
> controlPort << getconf bridge
>
> controlPort >> 250-Bridge=obfs3 [ip] [fp]
> 250-Bridge=obfs3 [ip] [fp]
> 250-Bridge=obfs3 [ip] [fp]
> 250-Bridge=obfs3 [ip] [fp]
> 250 Bridge=obfs3 [ip] [fp]
> }}}
>
> Note the final line without a `-`.
Thanks, I missed that one.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/17568#comment:7>
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