[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #13338 [Tor]: Rewrite tor-fw-helper in Go (or another memory-safe language)
#13338: Rewrite tor-fw-helper in Go (or another memory-safe language)
-----------------------------+------------------------------
Reporter: arma | Owner: yawning
Type: enhancement | Status: needs_review
Priority: minor | Milestone: Tor: unspecified
Component: Tor | Version:
Resolution: | Keywords: flashproxy
Actual Points: | Parent ID: #5213
Points: |
-----------------------------+------------------------------
Comment (by yawning):
Commenting on the stuff that doesn't require changes, fixes will happen
sometime over the weekend, maybe tonight if I decide not to go out.
Replying to [comment:18 dcf]:
> I was confused by this in [https://github.com/Yawning/go-fw-
helper/blob/4ffcbdde7063ac343e7f02a3e31c7243ec0f2bad/natclient/natpmp/packet.go
natclient/natpmp/packet.go]:
> {{{
> opRespOffset = 128
> ...
> if h.op != opExternalAddress+opRespOffset {
> }}}
> I think it would be more clear that you are checking the most-
significant bit if it were
> {{{
> opRespFlag = 0x80
> ...
> if h.op != opExternalAddress|opRespFlag {
> }}}
The NAT-PMP RFC defines responses the way it's laid out in the code in
their packet diagrams (Eg: "OP = 128 + 0"), though I may do what you
suggested here.
> What is this stuff at the bottom of some files?
> {{{
> var _ packetReq = (*externalAddressReq)(nil)
> var _ packetReq = (*requestMappingReq)(nil)
> }}}
It makes it immediately obvious that the concrete types do not implement
the base interface, as errors in the file where the concrete types are
defined. It's a development/debugging convenience for when the base
interface changes for any reason to make it immediately obvious that I
forgot to change the concrete implementations.
> What happens in [https://github.com/Yawning/go-fw-
helper/blob/4ffcbdde7063ac343e7f02a3e31c7243ec0f2bad/natclient/natpmp/getgateway_windows.go
natclient/natpmp/getgateway_windows.go] if the primary route is IPv6? It
looks like it at least won't crash; does it still do anything meaningful?
I'm not sure here as MSDN doesn't say what happens. I assume an error is
returned since the function definition and data structures involved all
specify IP addresses as `DWORDS`s. NAT-PMP as a protocol explicitly does
not support IPv6 (PCP does, but the protocol is more complex), so any
behavior here including (unlikely) getting a garbage return address is ok
since the actual discovery will fail.
> Technically, it seems like [https://github.com/Yawning/go-fw-
helper/blob/4ffcbdde7063ac343e7f02a3e31c7243ec0f2bad/natclient/upnp/httpu/httpu.go
httpu] should do [https://golang.org/pkg/net/http/#RoundTripper RoundTrip]
rather than [https://golang.org/pkg/net/http/#Client.Do Do], because
RoundTrip is for a single transaction and Do "follows policy (e.g.
redirects, cookies, auth)." That is, you might have to do less HTTP if you
introduce the UDP stuff at a lower layer (RoundTripper rather than
Client). But if it's at all awkward to do, then it doesn't matter; I'm
sure what's there will work perfectly well.
httpu's `Do` returns a slice of responses while the runtime's doesn't as
well, with the idea that the caller knows how to deal with getting
multiple responses, so the choice of `Do` here is more of a "trying to
make it easy to understand what is going on" rather than "implementing an
interface". I think things will be awkward regardless of what I do here,
so I'm inclined to leave it as is.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/13338#comment:19>
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