Skip to content

Commit

Permalink
Handle CHANNEL_EOF messages explicitly
Browse files Browse the repository at this point in the history
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
  • Loading branch information
slifty committed Feb 8, 2023
1 parent 3ce8d68 commit e74b0fb
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 3 deletions.
6 changes: 5 additions & 1 deletion src/classes/SshConnectionHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};

/**
Expand Down
29 changes: 27 additions & 2 deletions src/classes/SshSessionHandler.ts
Original file line number Diff line number Diff line change
@@ -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;
}

Expand Down Expand Up @@ -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 upstream has not yet patched the fix
this.session._channel.end(); // eslint-disable-line max-len, no-underscore-dangle, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
};
}

0 comments on commit e74b0fb

Please sign in to comment.