From 9e9ab602caefb9f864e2a5f0d159d1267276ecc3 Mon Sep 17 00:00:00 2001 From: Dominic Tarr Date: Sun, 9 Jun 2019 12:52:24 +0200 Subject: [PATCH 1/5] better test coverage, reuse tests for net and ws --- test/plugs.js | 112 ++++++++++++++++++++++++++++---------------------- 1 file changed, 63 insertions(+), 49 deletions(-) diff --git a/test/plugs.js b/test/plugs.js index e3031d0..2525752 100644 --- a/test/plugs.js +++ b/test/plugs.js @@ -312,34 +312,44 @@ tape('onion plug', function (t) { t.end() }) -tape('id of stream from server', function (t) { - check = function (id, cb) { - cb(null, true) - } - var close = combined.server(function (stream) { - var addr = combined.parse(stream.address) - t.ok(addr) - console.log('address as seen on server', addr) - t.equal(addr[0].name, 'net') - t.deepEqual(addr[1], combined.parse(combined.stringify())[1]) - - pull(stream.source, stream.sink) //echo - }, function (err) { - if(err) throw err - }, function () { - - combined.client(combined.stringify(), function (err, stream) { - if(err) throw err +function testServerId(combined, name, port) { + tape('id of stream from server', function (t) { + check = function (id, cb) { + cb(null, true) + } + var close = combined.server(function (stream) { + console.log('raw address on server:', stream.address) var addr = combined.parse(stream.address) - t.equal(addr[0].name, 'net') - t.equal(addr[0].port, 4848) + t.ok(addr) + console.log('address as seen on server', addr) + t.equal((addr[0].name || addr[0].protocol).replace(':', ''), name) t.deepEqual(addr[1], combined.parse(combined.stringify())[1]) - stream.source(true, function () { - close(function() {t.end()}) + + pull(stream.source, stream.sink) //echo + }, function (err) { + if(err) throw err + }, function () { + + combined.client(combined.stringify(), function (err, stream) { + if(err) throw err + var addr = combined.parse(stream.address) + t.equal((addr[0].name || addr[0].protocol).replace(':', ''), name) + t.equal(+addr[0].port, 4848) + t.deepEqual(addr[1], combined.parse(combined.stringify())[1]) + stream.source(true, function () { + close(function() {t.end()}) + }) }) }) }) -}) + +} + +testServerId(combined, 'net') +testServerId(combined_ws, 'ws') + + + function testAbort (name, combined) { @@ -350,7 +360,7 @@ function testAbort (name, combined) { var abort = combined.client(combined.stringify(), function (err, stream) { t.ok(err) - + console.error("CLIENT ABORTED", err) // NOTE: without the timeout, we try to close the server // before it actually started listening, which fails and then // the server keeps runnung, causing the next test to fail with EADDRINUSE @@ -363,37 +373,41 @@ function testAbort (name, combined) { }) abort() - }) } testAbort('combined', combined) testAbort('combined.ws', combined_ws) -tape('error should have client address on it', function (t) { -// return t.end() - check = function (id, cb) { - throw new Error('should never happen') - } - var close = combined.server(function (stream) { - throw new Error('should never happen') - }, function (err) { - var addr = err.address - t.ok(/^net\:/.test(err.address)) - t.ok(/\~shs\:/.test(err.address)) - //the shs address won't actually parse, because it doesn't have the key in it - //because the key is not known in a wrong number. - }, function () { - - //very unlikely this is the address, which will give a wrong number at the server. - var addr = combined.stringify().replace(/shs:......../, 'shs:XXXXXXXX') - combined.client(addr, function (err, stream) { - //client should see client auth rejected - t.ok(err) - console.log('Calling close') - close() // in this case, net.server.close(cb) never calls its cb, why? - t.end() +function testErrorAddress(combined, type) { + + tape('error should have client address on it:' + type, function (t) { + check = function (id, cb) { + throw new Error('should never happen') + } + var close = combined.server(function (stream) { + throw new Error('should never happen') + }, function (err) { + var addr = err.address + t.ok(err.address.indexOf(type) == 0) //net or ws + t.ok(/\~shs\:/.test(err.address)) + //the shs address won't actually parse, because it doesn't have the key in it + //because the key is not known in a wrong number. + }, function () { + + //very unlikely this is the address, which will give a wrong number at the server. + var addr = combined.stringify().replace(/shs:......../, 'shs:XXXXXXXX') + combined.client(addr, function (err, stream) { + //client should see client auth rejected + t.ok(err) + console.log('Calling close') + close() // in this case, net.server.close(cb) never calls its cb, why? + t.end() + }) }) }) -}) +} + +testErrorAddress(combined, 'net') +testErrorAddress(combined_ws, 'ws') From b0b79a3d2292e509d316aa10a009914bed3fdd40 Mon Sep 17 00:00:00 2001 From: Dominic Tarr Date: Sun, 9 Jun 2019 12:53:16 +0200 Subject: [PATCH 2/5] do not pass cb to stream.close, stream will call it's own close cb anyway --- plugins/ws.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/plugins/ws.js b/plugins/ws.js index be732a8..9a1fac9 100644 --- a/plugins/ws.js +++ b/plugins/ws.js @@ -96,7 +96,7 @@ module.exports = function (opts) { stream.address = addr return function () { - stream.close(cb) + stream.close() } }, stringify: function (scope) { @@ -127,7 +127,3 @@ module.exports = function (opts) { } } } - - - - From 2eb7192c8e92afa266e22468190c495467730372 Mon Sep 17 00:00:00 2001 From: Anders Rune Jensen Date: Tue, 1 Sep 2020 12:19:00 +0200 Subject: [PATCH 3/5] Fix wrong merge --- test/plugs.js | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/test/plugs.js b/test/plugs.js index 574bb3b..5ee9e01 100644 --- a/test/plugs.js +++ b/test/plugs.js @@ -415,28 +415,30 @@ function testAbort (name, combined) { testAbort('combined', combined) testAbort('combined.ws', combined_ws) -tape('error should have client address on it', function (t) { - // return t.end() - check = function (id, cb) { - throw new Error('should never happen') - } - var close = combined.server(function (stream) { - throw new Error('should never happen') - }, function (err) { - t.ok(/^net:/.test(err.address)) - t.ok(/~shs:/.test(err.address)) - //the shs address won't actually parse, because it doesn't have the key in it - //because the key is not known in a wrong number. - }, function () { - - //very unlikely this is the address, which will give a wrong number at the server. - var addr = combined.stringify().replace(/shs:......../, 'shs:XXXXXXXX') - combined.client(addr, function (err, stream) { - //client should see client auth rejected - t.ok(err) - console.log('Calling close') - close() // in this case, net.server.close(cb) never calls its cb, why? - t.end() +function testErrorAddress(combined, type) { + tape('error should have client address on it:' + type, function (t) { + check = function (id, cb) { + throw new Error('should never happen') + } + var close = combined.server(function (stream) { + throw new Error('should never happen') + }, function (err) { + var addr = err.address + t.ok(err.address.indexOf(type) == 0) //net or ws + t.ok(/\~shs\:/.test(err.address)) + //the shs address won't actually parse, because it doesn't have the key in it + //because the key is not known in a wrong number. + }, function () { + + //very unlikely this is the address, which will give a wrong number at the server. + var addr = combined.stringify().replace(/shs:......../, 'shs:XXXXXXXX') + combined.client(addr, function (err, stream) { + //client should see client auth rejected + t.ok(err) + console.log('Calling close') + close() // in this case, net.server.close(cb) never calls its cb, why? + t.end() + }) }) }) } From 8526dbafc055cdfb3236290d6577e2d8c75c2030 Mon Sep 17 00:00:00 2001 From: Anders Rune Jensen Date: Fri, 23 Apr 2021 01:25:49 +0200 Subject: [PATCH 4/5] Use pull-websocket and fix some tests --- package.json | 2 +- plugins/ws.js | 26 ++++++++++++++------------ test/plugs.js | 18 ++++++++++-------- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/package.json b/package.json index cae5426..50b989d 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "multicb": "^1.2.2", "multiserver-scopes": "^1.0.0", "pull-stream": "^3.6.1", - "pull-ws": "^3.3.0", + "pull-websocket": "^3.4.0", "secret-handshake": "^1.1.16", "separator-escape": "0.0.1", "socks": "^2.2.3", diff --git a/plugins/ws.js b/plugins/ws.js index c4f2ab9..d548b29 100644 --- a/plugins/ws.js +++ b/plugins/ws.js @@ -1,11 +1,11 @@ -var WS = require('pull-ws') +var WS = require('pull-websocket') var URL = require('url') var pull = require('pull-stream/pull') var Map = require('pull-stream/throughs/map') var scopes = require('multiserver-scopes') var http = require('http') -var https = require('https') -var fs = require('fs') +var https = require('https') +var fs = require('fs') var debug = require('debug')('multiserver:ws') function safe_origin (origin, address, port) { @@ -51,9 +51,7 @@ module.exports = function (opts = {}) { name: 'ws', scope: () => scope, server: function (onConnect, startedCb) { - if (WS.createServer == null) { - return null - } + if (WS.createServer == null) return null // Maybe weird: this sets a random port each time that `server()` is run // whereas the net plugin sets the port when the outer function is run. @@ -68,9 +66,10 @@ module.exports = function (opts = {}) { opts.cert = fs.readFileSync(opts.cert) var server = opts.server || - (opts.key && opts.cert ? https.createServer({ key: opts.key, cert: opts.cert }, opts.handler) : http.createServer(opts.handler)) + (opts.key && opts.cert ? https.createServer({ key: opts.key, cert: opts.cert }, opts.handler) : http.createServer(opts.handler)) - WS.createServer(Object.assign({}, opts, {server: server}), function (stream) { + const serverOpts = Object.assign({}, opts, {server: server}) + let wsServer = WS.createServer(serverOpts, function (stream) { stream.address = safe_origin( stream.headers.origin, stream.remoteAddress, @@ -90,10 +89,13 @@ module.exports = function (opts = {}) { return function (cb) { debug('Closing server on %s:%d', opts.host, opts.port) - server.close(function(err) { - if (err) console.error(err) - else debug('No longer listening on %s:%d', opts.host, opts.port) - if (cb) cb(err) + server.close((err) => { + wsServer.close(() => { + debug('after WS close', err) + if (err) console.error(err) + else debug('No longer listening on %s:%d', opts.host, opts.port) + if (cb) cb(err) + }) }) } }, diff --git a/test/plugs.js b/test/plugs.js index 39bd807..275b24b 100644 --- a/test/plugs.js +++ b/test/plugs.js @@ -378,10 +378,13 @@ function testServerId(combined, name, port) { if(err) throw err var addr = combined.parse(stream.address) t.equal((addr[0].name || addr[0].protocol).replace(':', ''), name) - t.equal(+addr[0].port, 4848) + if (addr[0].protocol === 'ws:') + t.equal(+addr[0].port, 4849) + else + t.equal(+addr[0].port, 4848) t.deepEqual(addr[1], combined.parse(combined.stringify())[1]) stream.source(true, function () { - close(function() {t.end()}) + close(t.end) }) }) }) @@ -411,7 +414,7 @@ function testAbort (name, combined) { // This is messy, combined.server should be a proper async call setTimeout( function() { console.log('Calling close') - close(function() {t.end()}) + close(t.end) }, 500) }) @@ -442,16 +445,15 @@ function testErrorAddress(combined, type) { combined.client(addr, function (err, stream) { //client should see client auth rejected t.ok(err) - console.log('Calling close') - close() // in this case, net.server.close(cb) never calls its cb, why? - t.end() + close(t.end) }) }) }) } testErrorAddress(combined, 'net') -testErrorAddress(combined_ws, 'ws') +// FIXME: ws server close is not working properly here +//testErrorAddress(combined_ws, 'ws') tape('multiple public different hosts', function(t) { var net1 = Net({ host: '127.0.0.1', port: 4854, scope: 'public'}) @@ -610,4 +612,4 @@ tape('ws: external is an array w/ multiple entries & shs transform', function (t 'ws://domain.de:9966~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=;ws://funtime.net:9966~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=' ) t.end() -}) \ No newline at end of file +}) From e647c6b1b581381eecefae030184f81e534f45df Mon Sep 17 00:00:00 2001 From: Anders Rune Jensen Date: Fri, 23 Apr 2021 14:53:58 +0200 Subject: [PATCH 5/5] Fix test for ws error addr --- plugins/ws.js | 12 +++++------- test/plugs.js | 11 +++++++---- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/plugins/ws.js b/plugins/ws.js index d548b29..5895757 100644 --- a/plugins/ws.js +++ b/plugins/ws.js @@ -89,13 +89,11 @@ module.exports = function (opts = {}) { return function (cb) { debug('Closing server on %s:%d', opts.host, opts.port) - server.close((err) => { - wsServer.close(() => { - debug('after WS close', err) - if (err) console.error(err) - else debug('No longer listening on %s:%d', opts.host, opts.port) - if (cb) cb(err) - }) + wsServer.close((err) => { + debug('after WS close', err) + if (err) console.error(err) + else debug('No longer listening on %s:%d', opts.host, opts.port) + if (cb) cb(err) }) } }, diff --git a/test/plugs.js b/test/plugs.js index 275b24b..d9cedfc 100644 --- a/test/plugs.js +++ b/test/plugs.js @@ -439,21 +439,24 @@ function testErrorAddress(combined, type) { //the shs address won't actually parse, because it doesn't have the key in it //because the key is not known in a wrong number. }, function () { - //very unlikely this is the address, which will give a wrong number at the server. var addr = combined.stringify().replace(/shs:......../, 'shs:XXXXXXXX') combined.client(addr, function (err, stream) { //client should see client auth rejected t.ok(err) - close(t.end) + close(() => { + if (type === 'ws') // we need to wait for the kill + setTimeout(t.end, 1100) + else + t.end() + }) }) }) }) } testErrorAddress(combined, 'net') -// FIXME: ws server close is not working properly here -//testErrorAddress(combined_ws, 'ws') +testErrorAddress(combined_ws, 'ws') tape('multiple public different hosts', function(t) { var net1 = Net({ host: '127.0.0.1', port: 4854, scope: 'public'})