Opened 7 years ago

Closed 6 years ago

Last modified 5 years ago

#3059 defect closed fixed (fixed)

twisted.internet.tcp.Client.getPeer incorrectly returns hostnames

Reported by: exarkun Owned by:
Priority: high Milestone:
Component: core Keywords:
Cc: Branch: branches/getpeer-hostname-3059-2
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

Description

If a hostname is passed to connectTCP, it will appear in the peer address. The IPv4Address should always have the IP address instead.

The IConnector returned by connectTCP seems to have the same problem with its getDestination method. This is a bit trickier, since the connector is available synchronously, whereas name resolution is asynchronous...

Change History (16)

comment:1 Changed 7 years ago by glyph

This highlights another deficiency in Connector as well; its job should be reserved for something like endpoints.

If getDestination is supposed to answer the question "where will this connector reconnect to" then it is fundamentally broken. Passing a hostname to connectTCP and then calling connect on the connector ought to reconnect to the hostname, including name resolution (honoring, for example, round-robin DNS if it is configured for that host).

Perhaps we should simply remove that method, or give it a different return type if a hostname was specified? (Returning an IPv4Address with a hostname in it is definitely wrong, no argument there.)

comment:2 Changed 6 years ago by exarkun

  • author set to exarkun
  • Branch set to branches/getpeer-hostname-3059

(In [23993]) Branching to 'getpeer-hostname-3059'

comment:3 Changed 6 years ago by exarkun

  • Keywords review added
  • Owner glyph deleted

Client.getPeer fixed

comment:4 Changed 6 years ago by therve

  • Keywords review removed
  • Owner set to exarkun

Please merge.

comment:5 Changed 6 years ago by therve

Oups, this fails under iocpreactor:

[FAIL]: twisted.internet.test.test_tcp.TCPClientTestsBuilder_IOCPReactor.test_addresses

Traceback (most recent call last):
  File "s:\buildbots\twisted\server-2k8-x86-py2.5-select\Twisted\twisted\internet\test\test_tcp.py", line 111, in test_addresses
    IPv4Address('TCP', '127.0.0.1', serverAddress.port))
twisted.trial.unittest.FailTest: not equal:
a = <twisted.internet.address.IPv4Address object at 0x01C7CE70>
b = <twisted.internet.address.IPv4Address object at 0x01C58270>

I guess this is because of useless code duplication :/

comment:6 Changed 6 years ago by exarkun

(In [24055]) realAddress

refs #3059

comment:8 Changed 6 years ago by exarkun

  • Branch changed from branches/getpeer-hostname-3059 to branches/getpeer-hostname-3059-2

(In [24057]) Branching to 'getpeer-hostname-3059-2'

comment:9 Changed 6 years ago by exarkun

  • Keywords review added
  • Owner exarkun deleted

Okay, that build failed. More recent results - <http://buildbot.twistedmatrix.com/builders/server-2k8-x86-py2.5-iocp/builds/88>, <http://buildbot.twistedmatrix.com/builders/winxp32-py2.5-iocp/builds/117>, including a fix to correctly propagate the right reactor in tcp.py that I pulled out of the #3218 branch.

comment:10 Changed 6 years ago by glyph

  • Keywords review removed
  • Owner set to exarkun

Among other things:

===============================================================================
[ERROR]: twisted.test.test_unix.UnixSocketTestCase.test_pidFile

Traceback (most recent call last):
  File "/home/glyph/Projects/Twisted/trunk/twisted/internet/tcp.py", line 834, in doRead
    transport = self.transport(skt, protocol, addr, self, s, self.reactor)
exceptions.TypeError: __init__() takes exactly 6 arguments (7 given)

comment:11 Changed 6 years ago by exarkun

(In [24089]) funny, I forgot to do this last time, too

refs #3059

comment:12 Changed 6 years ago by exarkun

  • Keywords review added
  • Owner exarkun deleted

comment:13 Changed 6 years ago by therve

  • Keywords review removed
  • Owner set to exarkun

Please merge.

comment:14 Changed 6 years ago by exarkun

  • Resolution set to fixed
  • Status changed from new to closed

(In [24132]) Merge getpeer-hostname-3059-2

Author: exarkun
Reviewer: glyph, therve
Fixes: #3059

Fix the implementations of getPeer for client transports so that
if the connection was made using a hostname, the returned IPv4Address
still contains an IPv4 address instead of the hostname.

comment:15 Changed 5 years ago by exarkun

This was a duplicate of #2738.

comment:16 Changed 4 years ago by <automation>

  • Owner exarkun deleted
Note: See TracTickets for help on using tickets.