Skip to content

Commit e1e89a0

Browse files
committed
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] mscdex/ssh2#1111
1 parent 3ce8d68 commit e1e89a0

File tree

2 files changed

+32
-3
lines changed

2 files changed

+32
-3
lines changed

src/classes/SshConnectionHandler.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,13 @@ export class SshConnectionHandler {
105105
): void => {
106106
logger.verbose('SSH request for a new session');
107107
const session = accept();
108-
const sessionHandler = new SshSessionHandler(this.authSession?.authToken ?? '');
108+
const sessionHandler = new SshSessionHandler(
109+
session,
110+
this.authSession?.authToken ?? '',
111+
);
109112
session.on('sftp', sessionHandler.onSftp);
110113
session.on('close', sessionHandler.onClose);
114+
session.on('eof', sessionHandler.onEof);
111115
};
112116

113117
/**

src/classes/SshSessionHandler.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,20 @@
11
import { logger } from '../logger';
22
import { SftpSessionHandler } from './SftpSessionHandler';
3-
import type { SFTPWrapper } from 'ssh2';
3+
import type {
4+
Session,
5+
SFTPWrapper,
6+
} from 'ssh2';
47

58
export class SshSessionHandler {
69
private readonly authToken: string;
710

8-
public constructor(authToken: string) {
11+
private readonly session: Session;
12+
13+
public constructor(
14+
session: Session,
15+
authToken: string,
16+
) {
17+
this.session = session;
918
this.authToken = authToken;
1019
}
1120

@@ -50,4 +59,20 @@ export class SshSessionHandler {
5059
public onClose = (): void => {
5160
logger.verbose('SSH session closed');
5261
};
62+
63+
public onEof = (): void => {
64+
// This addresses a bug in the ssh2 library where EOF is not properly
65+
// handled for sftp connections.
66+
// An upstream PR that would fix the behavior: https://github.com/mscdex/ssh2/pull/1111
67+
// And some context from our own debugging: https://github.com/PermanentOrg/sftp-service/issues/45
68+
//
69+
// The solution here is not ideal, as it is accessing an undocumented attribute that
70+
// doesn't exist in TypeScript. As a result I need to disable typescript checks.
71+
//
72+
// Once upstream makes that patch this entire handler should become completely unnecessary
73+
//
74+
// !!BEWARE: THERE BE DRAGONS HERE!!
75+
// @ts-expect-error because `_channel` is private / isn't actually documented.
76+
this.session._channel.end(); // eslint-disable-line max-len, no-underscore-dangle, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
77+
};
5378
}

0 commit comments

Comments
 (0)