Opened 7 years ago

Closed 2 years ago

#2997 defect closed fixed (fixed)

conch and getPeer

Reported by: bernat@… Owned by: z3p
Priority: normal Milestone:
Component: conch Keywords:
Cc: Branch: branches/conch-session-getPeer-2997
(diff, github, buildbot, log)
Author: z3p Launchpad Bug:

Description (last modified by thijs)

I would like to use getPeer from conch but there is no easy way of this. For example, if I take sshsimpleserver.py and apply the following patch:

  • sshsimpleserver.py

    old new  
    3030    """this is our example protocol that we will run over SSH 
    3131    """ 
    3232    def dataReceived(self, data): 
     33        print self.transport.getPeer() 
    3334        if data == '\r': 
    3435            data = '\r\n' 
    3536        elif data == '\x03': #^C 

I get:

        exceptions.AttributeError: SSHSessionProcessProtocol instance has no attribute 'getPeer'

The solution seems to be to use self.transport.session.getPeer(). I think that getPeer method should be implemented in SSHSessionProcessProtocol (to be able to use a protocol with SSH or without SSH for example).

In fact, there seems to be a bug in SSHChannel. When using self.transport.session.getPeer(), I get :

          File "/usr/lib/python2.4/site-packages/twisted/conch/ssh/channel.py", line 219, in getPeer
            return('SSH', )+self.conn.transport.getPeer()
        exceptions.AttributeError: SSHServerTransport instance has no attribute 'getPeer'

The fix should be to use self.conn.transport.transport.getPeer().

Change History (13)

comment:1 Changed 7 years ago by bernat@…

The first part of this bug is in fact already in #2453. Sorry for that. The second part is still valid though.

comment:2 Changed 3 years ago by z3p

  • Author set to z3p
  • Branch set to branches/conch-session-getPeer-2997

(In [33133]) Branching to 'conch-session-getPeer-2997'

comment:4 Changed 3 years ago by z3p

  • Keywords review added

Forgot the review keyword.

comment:5 Changed 3 years ago by z3p

  • Owner z3p deleted

I shouldn't review my own ticket.

comment:6 Changed 3 years ago by thijs

  • Description modified (diff)

comment:7 Changed 3 years ago by itamar

  • Keywords review removed
  • Owner set to z3p

Glad you're back to working on conch!

Major issues:

  1. The tests looked like they failed on the Windows build slaves.

Minor Issues:

  1. You're doing (in address.py) 'SSHTransportAddress(%r)' % self.address. Better to always do 'SSHTransportAddress(%r)' % (self.address,) in case some jerk decides to pass in a tuple.

Coding standard issues:

  1. You're missing a news file in twisted/conch/topfiles/.
  2. The new address test class should have a docstring.
  3. Two lines between each method in all the code you added, rather than one.
  4. In test docstrings you're using L{code} to refer to objects that don't exist at the given import path; L is supposed to be for links, to objects that are actually in that python module at that full or relative Python path. Use C{} for arbitrary code snippets and references. In practice this was in tests, where docstring correctness doesn't matter.
  5. Some lines in transport.py are longer than 80 chars.
  6. @ivar secondary lines (address.py) are usually indented, though I have no idea if that's actually required.

comment:8 Changed 3 years ago by exarkun

  • Owner changed from z3p to teratorn

#5566 was a duplicate of this.

comment:9 Changed 2 years ago by z3p

  • Keywords review added
  • Owner teratorn deleted

I believe I've addressed those review comments in the latest commit. I'm having trouble loading BuildBot at the moment, but I did start the builds: http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/conch-session-getPeer-2997

comment:10 Changed 2 years ago by therve

  • Keywords review removed
  • Owner set to z3p
  1. Pyflakes: twisted/conch/test/test_transport.py:573: local variable 'testCipher' is assigned to but never used
  1. It's better to only import SSHTransportAddress instead of the whole address module if you only use that object.
  1. Please add @since marker to your new module and your 2 new methods.
  1. Please put 3 lines after the imports and before your first class.

All very minor, please merge once fixed. Thanks!

comment:11 Changed 2 years ago by z3p

(In [34443]) comment 1: pyflakes/80cols

Refs #2997

comment:12 Changed 2 years ago by z3p

(In [34445]) comment 4: 3 lines between imports and first class

Refs #2997

comment:13 Changed 2 years ago by z3p

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

(In [34448]) Merge conch-session-getPeer-2997

Author: z3p
Reviewer: itamar, therve
Fixes: #2997

Implements getPeer()/getHost() for the SSHTransport. This also creates a new
SSHTransportAddress which contains the data for those methods.

Note: See TracTickets for help on using tickets.