Opened 8 years ago

Last modified 8 years ago

#5208 defect new

SOCKv4 expects t.i.t.Port.getHost() to return a tuple

Reported by: Peter Le Bek Owned by: Peter Le Bek
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch:


The BIND apparatus on SOCKv4 has to call getHost() on it's port to let a SOCKS client know where it's listening. It tries to slice the result of getHost() to return a (host, port) tuple - IPv4Address instances don't like being sliced!

This patch fixes the defect and also updates SOCKv4 to use its instance reactor instead of an imported one.

I'm not sure how to write tests for listenClass/connectClass, I think using an instance reactor at least makes this easier than it would have been, but I still need some direction (e.g. can I get rid of SOCKSv4Driver from the unit tests? use a custom reactor instead?).

Attachments (1)

socksv4-fix.patch (1.5 KB) - added by Peter Le Bek 8 years ago.

Download all attachments as: .zip

Change History (3)

Changed 8 years ago by Peter Le Bek

Attachment: socksv4-fix.patch added

comment:1 Changed 8 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to Peter Le Bek

Thanks! It looks like this might have only broken recently, when #4817 was resolved in Feb of this year. We would have caught it earlier if there were some unit tests, of course, so that's a very good direction to be considering. :)

Perhaps twisted.test.proto_helpers.MemoryReactor will help make it clear how listenClass/connectClass can be tested? It's basically a non-reactor reactor.

Apart from the testing issue, it would be nice to give some of these method docstrings explaining their purpose and the meaning of their parameters.


comment:2 Changed 8 years ago by Peter Le Bek

OK, cheers.

I'll check these too.

dune:twisted peter$ grep -r -i "getHost()\[" twisted/
twisted/conch/                portToBind = listener.getHost()[2] # the port
twisted/manhole/        host = ':'.join(map(str,[1:]))
twisted/protocols/        return transport.getHost()[2]
twisted/runner/        portNo = self._port.getHost()[2]
Note: See TracTickets for help on using tickets.