[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #8402 [Tor]: Tor should help its transport proxy use a proxy, if needed.
#8402: Tor should help its transport proxy use a proxy, if needed.
------------------------+--------------------------------------
Reporter: asn | Owner:
Type: defect | Status: needs_revision
Priority: normal | Milestone: Tor: 0.2.6.x-final
Component: Tor | Version:
Resolution: | Keywords: tor-bridge pt flashproxy
Actual Points: | Parent ID:
Points: |
------------------------+--------------------------------------
Comment (by asn):
Replying to [comment:16 yawning]:
> Replying to [comment:14 asn]:
> > A few comments from a preliminary review. I would like to review it
once again:
> >
> > - As we discussed in IRC, unit tests for the non-trivial additions
would be great.
>
> Will do.
>
> > - Maybe we could functionify the new duplicate code in
`get_proxy_addrport()`. I know that Nick hates duplicate code, and I share
his sentiments.
>
> Ditto. I would have thought of something clever to do, but I was
focused on getting an initial revision that worked.
>
> > - This is more of a comment to the original proposal, but isn't `PROXY
true` a bit off in a protocol that doesn't have any other `false`/`true`
strings? Maybe `PROXY DONE` is more appropriate? Maybe not.
>
> I would be ok with this, and agree that it makes sense (I considered
doing it when I was writing, but decided to implement that portion of the
spec as is).
>
Hm, if you also like the suggestion, let's do it then?
> > - `acked_proxy` is a bit of a deceiving name. Maybe we should change
the variable name to imply some connection to the proxy?
>
> Hmmm, I was going to change it to proxy_acked to match proxy_uri as far
as naming goes, if there is something better for "the pt claims it will
use the specified proxy", then I'm open to suggestions.
>
Hm, not sure. How about `supports_proxy`? So that you can do `if
(mp.supports_proxy) {`?
Doesn't look terribly bad. Probably can be improved.
> (Is what I said in comment #11 correct? The proposal should be changed
to not require the pluggable transport to verify that the proxy is
actually usable during the config right? Apart from trying to connect to
something, that's not possible with SOCKS4 or 5 in a way that isn't at
least somewhat suspicious.)
Yes, I think that's reasonable. The `PROXY true` message is there so that
Tor is assured that the PT understands that it has to use a proxy for any
outgoing connections. It doesn't seem necessary to test the proxy for
correctness at configure time.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/8402#comment:17>
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