Opened 7 years ago

Closed 3 years ago

#2453 defect closed fixed (fixed)

SSHSessionProcessProtocol does not implement getHost/getPeer.

Reported by: jrydberg Owned by: exarkun
Priority: normal Milestone:
Component: conch Keywords: soc2007 easy
Cc: Branch: branches/session-transport-addresses-2453
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

Description

session.SSHSessionProcessProtocol does not implement getHost/getPeer methods
of the transport interface.

Attachments (1)

twisted-2453.diff (4.1 KB) - added by salgado 3 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 7 years ago by z3p

  • Keywords soc2007 added

Tagging soc2007

comment:2 Changed 3 years ago by <automation>

  • Owner z3p deleted

comment:3 Changed 3 years ago by exarkun

  • Keywords easy added

These should return some sort of IAddress. An easy solution would be to just return the host/peer that the corresponding getHost or getPeer method on the wrapped session.conn.transport would return (of course it's somewhat gross to reach that far out of the SSHSessionProcessProtocol... but it's not exactly surprising in Conch).

comment:4 Changed 3 years ago by salgado

I'm probably missing something, but it confuses me that SSHSessionProcessProtocol should implement methods of the transport interface when it doesn't seem to provide ITransport?

comment:5 Changed 3 years ago by salgado

  • Owner set to salgado
  • Status changed from new to assigned

I discussed this with exarkun and it seems that SSHSessionProcessProtocol should be marked as implementing ITransport, so I may do that as well.

Changed 3 years ago by salgado

comment:6 Changed 3 years ago by salgado

This is my first attempt at adding getHost/getPeer to SSHSessionProcessProtocol. I couldn't resist adding/removing some blank lines to make the surroundings of what I changed follow the current policy (2 lines between methods and 3 between top-level classes is what I was told).

comment:7 Changed 3 years ago by salgado

  • Keywords review added
  • Owner salgado deleted
  • Status changed from assigned to new

comment:8 Changed 3 years ago by exarkun

  • Author set to exarkun
  • Branch set to branches/session-transport-addresses-2453

(In [31141]) Branching to 'session-transport-addresses-2453'

comment:9 Changed 3 years ago by exarkun

(In [31142]) Apply patch from the ticket

refs #2453

comment:10 Changed 3 years ago by exarkun

  • Keywords review removed
  • Owner set to exarkun
  • Status changed from new to assigned

Great, thanks! The tests and implementation look good, except for assertEqual which I'll change to assertEquals before merging. Build results also look good (at least, as good as I expect them to look at the moment). So I'll go ahead and apply this. Thanks again!

comment:11 Changed 3 years ago by exarkun

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

(In [31146]) Merge session-transport-addresses-2453

Author: salgado
Reviewer: exarkun
Fixes: #2453

Add getHost and getPeer methods to SSHSessionProcessProtocol, used as the transport
for subsystem protocols.

Note: See TracTickets for help on using tickets.