From 0c2bad766580094916518b13f8c9c9c0d1124e12 Mon Sep 17 00:00:00 2001 From: Dan Schultz Date: Wed, 8 Feb 2023 14:14:55 -0500 Subject: [PATCH] Handle CHANNEL_EOF messages explicitly There is an open PR in the upstream / ssh2 library related to properly closing SFTP channels when a `CHANNEL_EOF` message is received[1]. Unfortunately that PR has not yet been merged. Until that happens we need to handle the signal ourselves. Unfortunately this requires us to access "private" attributes (JavaScript doesn't support the concept of private attributes, so we are able to do this...). This is, of course a terrible and horrible idea and all of our tooling gets very upset about it. As a result I had to disable the tooling for the relevant line. Once the PR is merged in upstream we should upgrade to it and delete the contents of this commit. Issue #45 rclone hangs after completion [1] https://github.com/mscdex/ssh2/pull/1111 --- src/classes/SshConnectionHandler.ts | 6 +++++- src/classes/SshSessionHandler.ts | 29 +++++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/classes/SshConnectionHandler.ts b/src/classes/SshConnectionHandler.ts index 3e63ce8d..28af8fb9 100644 --- a/src/classes/SshConnectionHandler.ts +++ b/src/classes/SshConnectionHandler.ts @@ -105,9 +105,13 @@ export class SshConnectionHandler { ): void => { logger.verbose('SSH request for a new session'); const session = accept(); - const sessionHandler = new SshSessionHandler(this.authSession?.authToken ?? ''); + const sessionHandler = new SshSessionHandler( + session, + this.authSession?.authToken ?? '', + ); session.on('sftp', sessionHandler.onSftp); session.on('close', sessionHandler.onClose); + session.on('eof', sessionHandler.onEof); }; /** diff --git a/src/classes/SshSessionHandler.ts b/src/classes/SshSessionHandler.ts index 259b3147..9f3d09bb 100644 --- a/src/classes/SshSessionHandler.ts +++ b/src/classes/SshSessionHandler.ts @@ -1,11 +1,20 @@ import { logger } from '../logger'; import { SftpSessionHandler } from './SftpSessionHandler'; -import type { SFTPWrapper } from 'ssh2'; +import type { + Session, + SFTPWrapper, +} from 'ssh2'; export class SshSessionHandler { private readonly authToken: string; - public constructor(authToken: string) { + private readonly session: Session; + + public constructor( + session: Session, + authToken: string, + ) { + this.session = session; this.authToken = authToken; } @@ -50,4 +59,20 @@ export class SshSessionHandler { public onClose = (): void => { logger.verbose('SSH session closed'); }; + + public onEof = (): void => { + // This addresses a bug in the ssh2 library where EOF is not properly + // handled for sftp connections. + // An upstream PR that would fix the behavior: https://github.com/mscdex/ssh2/pull/1111 + // And some context from our own debugging: https://github.com/PermanentOrg/sftp-service/issues/45 + // + // The solution here is not ideal, as it is accessing an undocumented attribute that + // doesn't exist in TypeScript. As a result I need to disable typescript checks. + // + // Once upstream makes that patch this entire handler should become completely unnecessary + // + // !!BEWARE: THERE BE DRAGONS HERE!! + // @ts-expect-error because `_channel` is private / isn't actually documented. + this.session._channel.end(); // eslint-disable-line max-len, no-underscore-dangle, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access + }; }