Skip to content

Commit 2b2f103

Browse files
authored
Merge pull request #15 from mailsvb/fixPasvPorts
add timeout to data sockets
2 parents f70bcf7 + e3196c1 commit 2b2f103

4 files changed

Lines changed: 57 additions & 3 deletions

File tree

lib/jsftpd.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -959,6 +959,11 @@ class ftpd {
959959
if (isSecure === true && protection === true) {
960960
dataChannel = tls.Server(main._opt.tls)
961961
dataChannel.on('secureConnection', (pasvSocket) => {
962+
pasvSocket.on('error', main.ErrorHandler)
963+
pasvSocket.setTimeout(5000)
964+
pasvSocket.on('timeout', () => {
965+
pasvSocket.destroy()
966+
})
962967
main.DebugHandler(`${connectionInfo} data connection established`)
963968
dataObj.dataSocket = pasvSocket
964969
if (dataObj.method) {
@@ -968,6 +973,12 @@ class ftpd {
968973
} else {
969974
dataChannel = net.Server()
970975
dataChannel.on('connection', (pasvSocket) => {
976+
pasvSocket.on('error', main.ErrorHandler)
977+
pasvSocket.setTimeout(5000)
978+
pasvSocket.on('timeout', () => {
979+
pasvSocket.destroy()
980+
})
981+
pasvSocket.on('close', () => main.DebugHandler(`${connectionInfo} data connection has been closed`))
971982
if (isSecure === true && protection === true) {
972983
pasvSocket = new tls.TLSSocket(pasvSocket, { isServer: true, secureContext: tls.createSecureContext(main._opt.tls) })
973984
pasvSocket.on('secure', () => {
@@ -1004,11 +1015,16 @@ class ftpd {
10041015
dataObj.dataSocket = activeSocket
10051016
dataObj.method(dataObj)
10061017
})
1018+
activeSocket.on('error', main.ErrorHandler)
10071019
} else {
10081020
obj.dataSocket = client
10091021
obj.method(obj)
10101022
}
10111023
})
1024+
client.setTimeout(5000)
1025+
client.on('timeout', () => {
1026+
client.destroy()
1027+
})
10121028
client.on('error', main.ErrorHandler)
10131029
} else {
10141030
obj.method(obj)

test/LIST.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ test('test LIST message', async () => {
3535

3636
let promiseSocket = new PromiseSocket(new net.Socket())
3737
let socket = promiseSocket.stream
38-
await socket.connect(cmdPortTCP, 'localhost')
38+
await socket.connect(cmdPortTCP, '127.0.0.1', 'localhost')
3939
content = await promiseSocket.read()
4040
expect(content.toString().trim()).toBe('220 Welcome')
4141

test/PASV.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ test('test PASV message takes next free port', async () => {
4040

4141
let promiseSocket = new PromiseSocket(new net.Socket())
4242
let socket = promiseSocket.stream
43-
await socket.connect(cmdPortTCP, 'localhost')
43+
await socket.connect(cmdPortTCP, '127.0.0.1', 'localhost')
4444
content = await promiseSocket.read()
4545
expect(content.toString().trim()).toBe('220 Welcome')
4646

test/STOR.test.js

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ const tls = require('tls')
44
const {PromiseSocket, TimeoutError} = require('promise-socket')
55
const { sleep, getCmdPortTCP, getDataPort } = require('./utils')
66

7-
jest.setTimeout(5000)
7+
jest.setTimeout(7500)
88
let server, content, dataContent = null
99
const cmdPortTCP = getCmdPortTCP()
1010
const dataPort = getDataPort()
@@ -128,6 +128,44 @@ test('test STOR message', async () => {
128128
await promiseSocket.end()
129129
})
130130

131+
test('test STOR message failes due to socket timeout', async () => {
132+
const users = [
133+
{
134+
username: 'john',
135+
allowLoginWithoutPassword: true,
136+
}
137+
]
138+
server = new ftpd({cnf: {port: 50021, user: users, minDataPort: dataPort}})
139+
expect(server).toBeInstanceOf(ftpd)
140+
server.start()
141+
142+
let promiseSocket = new PromiseSocket(new net.Socket())
143+
let socket = promiseSocket.stream
144+
await socket.connect(50021, 'localhost')
145+
content = await promiseSocket.read()
146+
expect(content.toString().trim()).toBe('220 Welcome')
147+
148+
await promiseSocket.write('USER john')
149+
content = await promiseSocket.read()
150+
expect(content.toString().trim()).toBe('232 User logged in')
151+
152+
await promiseSocket.write('STOR ../../mytestfile')
153+
content = await promiseSocket.read()
154+
expect(content.toString().trim()).toBe('550 Transfer failed "../../mytestfile"')
155+
156+
await promiseSocket.write('EPSV')
157+
content = await promiseSocket.read()
158+
expect(content.toString().trim()).toBe(`229 Entering extended passive mode (|||${dataPort}|)`)
159+
160+
let promiseDataSocket = new PromiseSocket(new net.Socket())
161+
let dataSocket = promiseDataSocket.stream
162+
await dataSocket.connect(dataPort, 'localhost')
163+
164+
await sleep(5500)
165+
expect(dataSocket.destroyed).toBe(true)
166+
167+
await promiseSocket.end()
168+
})
131169

132170
test('test STOR message with ASCII', async () => {
133171
const users = [

0 commit comments

Comments
 (0)