Opened 11 years ago

Closed 6 years ago

#2453 defect closed fixed (fixed)

SSHSessionProcessProtocol does not implement getHost/getPeer.

Reported by: jrydberg Owned by: Jean-Paul Calderone
Priority: normal Milestone:
Component: conch Keywords: soc2007 easy
Cc: Branch: branches/session-transport-addresses-2453
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

Description

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

Attachments (1)

twisted-2453.diff (4.1 KB) - added by Guilherme Salgado 6 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 10 years ago by z3p

Keywords: soc2007 added

Tagging soc2007

comment:2 Changed 7 years ago by <automation>

Owner: z3p deleted

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

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 6 years ago by Guilherme 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 6 years ago by Guilherme Salgado

Owner: set to Guilherme Salgado
Status: newassigned

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

Changed 6 years ago by Guilherme Salgado

Attachment: twisted-2453.diff added

comment:6 Changed 6 years ago by Guilherme 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 6 years ago by Guilherme Salgado

Keywords: review added
Owner: Guilherme Salgado deleted
Status: assignednew

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

Author: exarkun
Branch: branches/session-transport-addresses-2453

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

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

(In [31142]) Apply patch from the ticket

refs #2453

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

Keywords: review removed
Owner: set to Jean-Paul Calderone
Status: newassigned

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 6 years ago by Jean-Paul Calderone

Resolution: fixed
Status: assignedclosed

(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.