Skip to content

Commit f8f625f

Browse files
ajkrjEugeny
andauthored
fix(ssh): add EOF handling and error propagation for port forwarding (#10964)
Co-authored-by: Eugene <inbox@null.page>
1 parent 431ea21 commit f8f625f

File tree

1 file changed

+57
-21
lines changed

1 file changed

+57
-21
lines changed

tabby-ssh/src/session/ssh.ts

Lines changed: 57 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -495,10 +495,9 @@ export class SSHSession {
495495
this.emitServiceMessage(colors.bgRed.black(' X ') + ` Could not forward the remote connection to ${forward.targetAddress}:${forward.targetPort}: ${e}`)
496496
channel.close()
497497
})
498-
channel.data$.subscribe(data => socket.write(data))
499-
socket.on('data', data => channel.write(Uint8Array.from(data)))
500-
channel.closed$.subscribe(() => socket.destroy())
501-
socket.on('close', () => channel.close())
498+
499+
this.setupSocketChannelEvents(channel, socket, 'Remote forward')
500+
502501
socket.on('connect', () => {
503502
this.logger.info('Connection forwarded')
504503
})
@@ -519,19 +518,7 @@ export class SSHSession {
519518
try {
520519
const x11Stream = await socket.connect(displaySpec)
521520
this.logger.info('Connection forwarded')
522-
523-
channel.data$.subscribe(data => {
524-
x11Stream.write(data)
525-
})
526-
x11Stream.on('data', data => {
527-
channel.write(Uint8Array.from(data))
528-
})
529-
channel.closed$.subscribe(() => {
530-
socket.destroy()
531-
})
532-
x11Stream.on('close', () => {
533-
channel.close()
534-
})
521+
this.setupSocketChannelEvents(channel, x11Stream, 'X11 forward')
535522
} catch (e) {
536523
// eslint-disable-next-line @typescript-eslint/no-base-to-string
537524
this.emitServiceMessage(colors.bgRed.black(' X ') + ` Could not connect to the X server: ${e}`)
@@ -788,10 +775,8 @@ export class SSHSession {
788775
throw err
789776
}))
790777
const socket = accept()
791-
channel.data$.subscribe(data => socket.write(data))
792-
socket.on('data', data => channel.write(Uint8Array.from(data)))
793-
channel.closed$.subscribe(() => socket.destroy())
794-
socket.on('close', () => channel.close())
778+
779+
this.setupSocketChannelEvents(channel, socket, 'Local forward')
795780
}).then(() => {
796781
this.emitServiceMessage(colors.bgGreen.black(' -> ') + ` Forwarded ${fw}`)
797782
this.forwardedPorts.push(fw)
@@ -865,6 +850,57 @@ export class SSHSession {
865850
return ch
866851
}
867852

853+
private setupSocketChannelEvents (channel: russh.Channel, socket: Socket, logPrefix: string): void {
854+
// Channel → Socket data flow with error handling
855+
channel.data$.subscribe({
856+
next: data => socket.write(data),
857+
error: err => {
858+
this.logger.error(`${logPrefix}: channel data error: ${err}`)
859+
socket.destroy()
860+
},
861+
})
862+
863+
// Socket → Channel data flow with proper conversion
864+
socket.on('data', data => {
865+
try {
866+
channel.write(new Uint8Array(data.buffer, data.byteOffset, data.byteLength))
867+
} catch (err) {
868+
this.logger.error(`${logPrefix}: channel write error: ${err}`)
869+
socket.destroy(new Error(`${logPrefix}failed to write to channel: ${err}`))
870+
}
871+
})
872+
873+
// Handle EOF from remote
874+
channel.eof$.subscribe(() => {
875+
this.logger.debug(`${logPrefix}: channel EOF received, ending socket`)
876+
socket.end()
877+
})
878+
879+
// Handle channel close
880+
channel.closed$.subscribe(() => {
881+
this.logger.debug(`${logPrefix}: channel closed, destroying socket`)
882+
socket.destroy()
883+
})
884+
885+
// Handle socket errors
886+
socket.on('error', err => {
887+
this.logger.error(`${logPrefix}: socket error: ${err}`)
888+
channel.close()
889+
})
890+
891+
// Handle socket close
892+
socket.on('close', () => {
893+
this.logger.debug(`${logPrefix}: socket closed, closing channel`)
894+
channel.close()
895+
})
896+
897+
// Handle EOF from local
898+
socket.on('end', () => {
899+
this.logger.debug(`${logPrefix}: socket end, sending EOF to channel`)
900+
channel.eof()
901+
})
902+
}
903+
868904
async loadPrivateKey (name: string, privateKeyContents: Buffer): Promise<russh.KeyPair> {
869905
this.activePrivateKey = await this.loadPrivateKeyWithPassphraseMaybe(privateKeyContents.toString())
870906
return this.activePrivateKey

0 commit comments

Comments
 (0)