Opened 5 years ago

Last modified 5 years ago

#5208 defect new

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

Reported by: lebek Owned by: lebek
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 lebek 5 years ago.

Download all attachments as: .zip

Change History (3)

Changed 5 years ago by lebek

comment:1 Changed 5 years ago by exarkun

  • Keywords review removed
  • Owner set to lebek

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 5 years ago by lebek

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.