Opened 15 years ago

Closed 15 years ago

#2002 defect closed fixed (fixed)

t.i.unix.Port fails on bound incoming unix sockets

Reported by: ghazel Owned by: ghazel
Priority: highest Milestone:
Component: core Keywords:
Cc: Jean-Paul Calderone, itamarst Branch:
Author:

Description

unix.Port specifically breaks connections from unix domain sockets that are bound.

t.i.u.Port:

    def _buildAddr(self, name):
        assert not name
        return None

Change History (10)

comment:1 Changed 15 years ago by ghazel

Keywords: review added

unixpeer-2002 with unittest

comment:2 Changed 15 years ago by ghazel

Cc: Jean-Paul Calderone itamarst added

After this, bindAddress support should be added to connectUNIX.

comment:3 Changed 15 years ago by ghazel

It looks like something is wrong on OS X with this branch..

Is there any reason twisted.internet.unix would not be used by OS X?

comment:4 Changed 15 years ago by Jean-Paul Calderone

Keywords: review removed

Dropping review keyword until things are sorted out on OS X

comment:5 Changed 15 years ago by ghazel

Keywords: review added

Works on OS X now.

comment:6 Changed 15 years ago by Jean-Paul Calderone

Owner: Glyph deleted
Priority: normalhighest

comment:7 Changed 15 years ago by Jean-Paul Calderone

Owner: set to Jean-Paul Calderone
Status: newassigned

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

Keywords: review removed
Owner: changed from Jean-Paul Calderone to ghazel
Status: assignednew

testPeerBind should have a docstring describing the feature it is testing.

I think self._addPorts(l) should be called regardless of whether the factory's Deferred fires successfully. If it isn't, if the test fails, it will also leak a port object. Likewise for the socket object the test creates. done should probably be addBoth'd to the Deferred (and passthrough its argument, in case it is failure).

It's generally better if asserts are lexically contained by their test method, but since that doesn't seem to be the style in test_unix, the way you have written the test is okay. Something to keep in mind in general, though.

Go ahead and merge after adding a docstring and fixing the cleanup issues.

comment:9 Changed 15 years ago by ghazel

The other tests also use addCallback, I was mimicing those.

I added the cleanup, but the other tests should be reviewed if this is actually an issue.

comment:10 Changed 15 years ago by alus

Resolution: fixed
Status: newclosed

(In [18009]) unix.Port getPeer support

Author: ghazel

Reviewer: exarkun

Fixes #2002

unix domain sockets can now get the remote endpoint that the connecting side is bound to using the normal method: transport.getPeer(). includes unittest

Note: See TracTickets for help on using tickets.