Skip to content

Commit

Permalink
fix(connector): build passive server correctly for tls (QuorumDMS#106)
Browse files Browse the repository at this point in the history
* fix(connector): build passive server correctly for tls

Fixes an issue where passive tls connections would never be fulfilled.

This uses `tls` to create a server if the connection is secure, which allows an ftp client to connect correctly

* test: stop skipping explicit tests
  • Loading branch information
trs authored Aug 9, 2018
1 parent c970a42 commit 66fc66e
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 13 deletions.
16 changes: 5 additions & 11 deletions src/connector/passive.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,7 @@ class Passive extends Connector {
}
this.log.trace({port, remoteAddress: socket.remoteAddress}, 'Passive connection fulfilled.');

if (this.connection.secure) {
const secureContext = tls.createSecureContext(this.server._tls);
const secureSocket = new tls.TLSSocket(socket, {
isServer: true,
secureContext
});
this.dataSocket = secureSocket;
} else {
this.dataSocket = socket;
}
this.dataSocket = socket;
this.dataSocket.connected = true;
this.dataSocket.setEncoding(this.connection.transferType);
this.dataSocket.on('error', err => this.server && this.server.emit('client-error', {connection: this.connection, context: 'dataSocket', error: err}));
Expand All @@ -68,8 +59,11 @@ class Passive extends Connector {
};

this.dataSocket = null;
this.dataServer = net.createServer({pauseOnConnect: true}, connectionHandler);

const serverOptions = Object.assign({}, this.connection.secure ? this.server._tls : {}, {pauseOnConnect: true});
this.dataServer = (this.connection.secure ? tls : net).createServer(serverOptions, connectionHandler);
this.dataServer.maxConnections = 1;

this.dataServer.on('error', err => this.server && this.server.emit('client-error', {connection: this.connection, context: 'dataServer', error: err}));
this.dataServer.on('close', () => {
this.log.trace('Passive server closed');
Expand Down
2 changes: 1 addition & 1 deletion src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class FtpServer extends EventEmitter {
return connection.reply(220, ...greeting, features)
.finally(() => socket.resume());
};
const serverOptions = _.assign(this.isTLS ? this._tls : {}, {pauseOnConnect: true});
const serverOptions = Object.assign({}, this.isTLS ? this._tls : {}, {pauseOnConnect: true});

this.server = (this.isTLS ? tls : net).createServer(serverOptions, serverConnectionHandler);
this.server.on('error', err => this.log.error(err, '[Event] error'));
Expand Down
2 changes: 1 addition & 1 deletion test/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ describe('Integration', function () {
runFileSystemTests('binary');
});

describe.skip('#EXPLICIT', function () {
describe('#EXPLICIT', function () {
before(() => {
return server.close()
.then(() => startServer('ftp://127.0.0.1:8880', {
Expand Down

0 comments on commit 66fc66e

Please sign in to comment.