diff --git a/app/api.js b/app/api.js index 29237c05..d7e5c215 100644 --- a/app/api.js +++ b/app/api.js @@ -147,7 +147,16 @@ function asyncInitWebSocket(server) { function listenForResponse(ws, canceller) { return new Promise((resolve, reject) => { + function handleClose(event) { + // a 'close' event before a 'message' event means the request failed + ws.removeEventListener('message', handleMessage); + const error = canceller.cancelled + ? canceller.error + : new Error('connection closed'); + reject(error); + } function handleMessage(msg) { + ws.removeEventListener('close', handleClose); try { const response = JSON.parse(msg.data); if (response.error) { @@ -156,13 +165,11 @@ function listenForResponse(ws, canceller) { resolve(response); } } catch (e) { - ws.close(); - canceller.cancelled = true; - canceller.error = e; reject(e); } } ws.addEventListener('message', handleMessage, { once: true }); + ws.addEventListener('close', handleClose, { once: true }); }); } @@ -215,7 +222,10 @@ async function upload( onprogress(size); size += buf.length; state = await reader.read(); - while (ws.bufferedAmount > ECE_RECORD_SIZE * 2) { + while ( + ws.bufferedAmount > ECE_RECORD_SIZE * 2 && + ws.readyState === WebSocket.OPEN + ) { await delay(); } } diff --git a/package-lock.json b/package-lock.json index abcf8208..0a0e5fe0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1243,6 +1243,25 @@ "minimalistic-crypto-utils": "^1.0.0" } }, + "@dannycoates/express-ws": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/@dannycoates/express-ws/-/express-ws-5.0.1.tgz", + "integrity": "sha512-VvSS796i/QMP2iqRO9Q+0r8UxoLKFH+moyX0GEf28eyjGSszyHM1GxdPwuEjtsOHtzk5QnrfB50ln2P9loJxvA==", + "requires": { + "esm": "^3.0.84", + "ws": "^7.1.1" + }, + "dependencies": { + "ws": { + "version": "7.1.1", + "resolved": "https://registry.npmjs.org/ws/-/ws-7.1.1.tgz", + "integrity": "sha512-o41D/WmDeca0BqYhsr3nJzQyg9NF5X8l/UdnFNux9cS3lwB+swm8qGWX5rn+aD6xfBU3rGmtHij7g7x6LxFU3A==", + "requires": { + "async-limiter": "^1.0.0" + } + } + } + }, "@dannycoates/webcrypto-liner": { "version": "0.1.37", "resolved": "https://registry.npmjs.org/@dannycoates/webcrypto-liner/-/webcrypto-liner-0.1.37.tgz", @@ -5986,14 +6005,6 @@ } } }, - "express-ws": { - "version": "github:dannycoates/express-ws#d0910a43b1802b22476362113557e20b18e185ba", - "from": "github:dannycoates/express-ws", - "requires": { - "esm": "^3.0.84", - "ws": "github:dannycoates/ws" - } - }, "extend": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/extend/-/extend-3.0.2.tgz", @@ -9649,9 +9660,10 @@ } }, "lodash": { - "version": "4.17.11", - "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.11.tgz", - "integrity": "sha512-cQKh8igo5QUhZ7lg38DYWAxMvjSAKG0A8wGSVimP07SIUEK2UO+arSRKbRZWtelMtN5V0Hkwh5ryOto/SshYIg==" + "version": "4.17.15", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.15.tgz", + "integrity": "sha512-8xOcRHvCjnocdS5cpwXQXVzmmh5e5+saE2QGoeQmbKmRS6J3VQppPOIt0MnmE+4xlZoumy0GPG0D0MVIQbNA1A==", + "dev": true }, "lodash._reinterpolate": { "version": "3.0.0", @@ -10274,9 +10286,9 @@ } }, "mixin-deep": { - "version": "1.3.1", - "resolved": "https://registry.npmjs.org/mixin-deep/-/mixin-deep-1.3.1.tgz", - "integrity": "sha512-8ZItLHeEgaqEvd5lYBXfm4EZSFCX29Jb9K+lAHhDKzReKBQKj3R+7NOF6tjqYi9t4oI8VUfaWITJQm86wnXGNQ==", + "version": "1.3.2", + "resolved": "https://registry.npmjs.org/mixin-deep/-/mixin-deep-1.3.2.tgz", + "integrity": "sha512-WRoDn//mXBiJ1H40rqa3vH0toePwSsGb45iInWlTySa+Uu4k3tYUSxa2v1KqAiLtvlrSzaExqS1gtk96A9zvEA==", "dev": true, "requires": { "for-in": "^1.0.2", @@ -14265,6 +14277,11 @@ "pend": "~1.2.0" } }, + "lodash": { + "version": "4.17.15", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.15.tgz", + "integrity": "sha512-8xOcRHvCjnocdS5cpwXQXVzmmh5e5+saE2QGoeQmbKmRS6J3VQppPOIt0MnmE+4xlZoumy0GPG0D0MVIQbNA1A==" + }, "ms": { "version": "2.1.1", "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.1.tgz", @@ -14446,9 +14463,9 @@ "dev": true }, "set-value": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/set-value/-/set-value-2.0.0.tgz", - "integrity": "sha512-hw0yxk9GT/Hr5yJEYnHNKYXkIA8mVJgd9ditYZCe16ZczcaELYYcfvaXesNACk2O8O0nTiPQcQhGUQj8JLzeeg==", + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/set-value/-/set-value-2.0.1.tgz", + "integrity": "sha512-JxHc1weCN68wRY0fhCoXpyK55m/XPHafOmK4UWD7m2CI14GMcFypt4w/0+NV5f/ZMby2F6S2wwA7fgynh9gWSw==", "dev": true, "requires": { "extend-shallow": "^2.0.1", @@ -16271,11 +16288,6 @@ } } }, - "ultron": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/ultron/-/ultron-1.1.1.tgz", - "integrity": "sha512-UIEXBNeYmKptWH6z8ZnqTeS8fV74zG0/eRU9VGkpzz+LIJNs8W/zM/L+7ctCkRrgbNnnR0xxw4bKOr0cW0N0Og==" - }, "unassert": { "version": "1.5.1", "resolved": "https://registry.npmjs.org/unassert/-/unassert-1.5.1.tgz", @@ -16354,38 +16366,15 @@ } }, "union-value": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/union-value/-/union-value-1.0.0.tgz", - "integrity": "sha1-XHHDTLW61dzr4+oM0IIHulqhrqQ=", + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/union-value/-/union-value-1.0.1.tgz", + "integrity": "sha512-tJfXmxMeWYnczCVs7XAEvIV7ieppALdyepWMkHkwciRpZraG/xwT+s2JN8+pr1+8jCRf80FFzvr+MpQeeoF4Xg==", "dev": true, "requires": { "arr-union": "^3.1.0", "get-value": "^2.0.6", "is-extendable": "^0.1.1", - "set-value": "^0.4.3" - }, - "dependencies": { - "extend-shallow": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/extend-shallow/-/extend-shallow-2.0.1.tgz", - "integrity": "sha1-Ua99YUrZqfYQ6huvu5idaxxWiQ8=", - "dev": true, - "requires": { - "is-extendable": "^0.1.0" - } - }, - "set-value": { - "version": "0.4.3", - "resolved": "https://registry.npmjs.org/set-value/-/set-value-0.4.3.tgz", - "integrity": "sha1-fbCPnT0i3H945Trzw79GZuzfzPE=", - "dev": true, - "requires": { - "extend-shallow": "^2.0.1", - "is-extendable": "^0.1.1", - "is-plain-object": "^2.0.1", - "to-object-path": "^0.3.0" - } - } + "set-value": "^2.0.1" } }, "uniq": { @@ -17692,31 +17681,6 @@ "integrity": "sha512-nqHUnMXmBzT0w570r2JpJxfiSD1IzoI+HGVdd3aZ0yNi3ngvQ4jv1dtHt5VGxfI2yj5yqImPhOK4vmIh2xMbGg==", "dev": true }, - "websocket-stream": { - "version": "5.5.0", - "resolved": "https://registry.npmjs.org/websocket-stream/-/websocket-stream-5.5.0.tgz", - "integrity": "sha512-EXy/zXb9kNHI07TIMz1oIUIrPZxQRA8aeJ5XYg5ihV8K4kD1DuA+FY6R96HfdIHzlSzS8HiISAfrm+vVQkZBug==", - "requires": { - "duplexify": "^3.5.1", - "inherits": "^2.0.1", - "readable-stream": "^2.3.3", - "safe-buffer": "^5.1.2", - "ws": "^3.2.0", - "xtend": "^4.0.0" - }, - "dependencies": { - "ws": { - "version": "3.3.3", - "resolved": "https://registry.npmjs.org/ws/-/ws-3.3.3.tgz", - "integrity": "sha512-nnWLa/NwZSt4KQJu51MYlCcSQ5g7INpOrOMt4XV8j4dqTXdmlUmSHQ8/oLC069ckre0fRsgfvsKwbTdtKLCDkA==", - "requires": { - "async-limiter": "~1.0.0", - "safe-buffer": "~5.1.0", - "ultron": "~1.1.0" - } - } - } - }, "wgxpath": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/wgxpath/-/wgxpath-1.0.0.tgz", @@ -17819,8 +17783,9 @@ } }, "ws": { - "version": "github:dannycoates/ws#c83cbb3bce478122cedcb8c475d9e86e1112824a", - "from": "github:dannycoates/ws", + "version": "6.1.2", + "resolved": "github:dannycoates/ws#c83cbb3bce478122cedcb8c475d9e86e1112824a", + "dev": true, "requires": { "async-limiter": "~1.0.0" } diff --git a/package.json b/package.json index e54ea2bc..694e1f27 100644 --- a/package.json +++ b/package.json @@ -137,6 +137,7 @@ "webpack-unassert-loader": "^1.2.0" }, "dependencies": { + "@dannycoates/express-ws": "^5.0.1", "@fluent/bundle": "^0.13.0", "@fluent/langneg": "^0.3.0", "@google-cloud/storage": "^3.0.4", @@ -146,7 +147,6 @@ "cldr-core": "^35.1.0", "convict": "^5.1.0", "express": "^4.17.1", - "express-ws": "github:dannycoates/express-ws", "fxa-geodb": "^1.0.4", "helmet": "^3.20.0", "mkdirp": "^0.5.1", @@ -155,8 +155,7 @@ "raven": "^2.6.4", "redis": "^2.8.0", "selenium-standalone": "^6.15.6", - "ua-parser-js": "^0.7.20", - "websocket-stream": "^5.5.0" + "ua-parser-js": "^0.7.20" }, "availableLanguages": [ "en-US", diff --git a/server/bin/dev.js b/server/bin/dev.js index de9b1b08..f1a1dec1 100644 --- a/server/bin/dev.js +++ b/server/bin/dev.js @@ -3,7 +3,7 @@ const routes = require('../routes'); const pages = require('../routes/pages'); const tests = require('../../test/frontend/routes'); const express = require('express'); -const expressWs = require('express-ws'); +const expressWs = require('@dannycoates/express-ws'); const morgan = require('morgan'); const config = require('../config'); diff --git a/server/bin/prod.js b/server/bin/prod.js index 56e35f1b..5f509dc3 100644 --- a/server/bin/prod.js +++ b/server/bin/prod.js @@ -4,7 +4,7 @@ const Raven = require('raven'); const config = require('../config'); const routes = require('../routes'); const pages = require('../routes/pages'); -const expressWs = require('express-ws'); +const expressWs = require('@dannycoates/express-ws'); if (config.sentry_dsn) { Raven.config(config.sentry_dsn).install(); diff --git a/server/bin/test.js b/server/bin/test.js index 9a4fa5e3..5cbed848 100644 --- a/server/bin/test.js +++ b/server/bin/test.js @@ -2,7 +2,7 @@ const assets = require('../../common/assets'); const routes = require('../routes'); const pages = require('../routes/pages'); const tests = require('../../test/frontend/routes'); -const expressWs = require('express-ws'); +const expressWs = require('@dannycoates/express-ws'); module.exports = function(app, devServer) { assets.setMiddleware(devServer.middleware); diff --git a/server/routes/ws.js b/server/routes/ws.js index c893c99f..32ea7905 100644 --- a/server/routes/ws.js +++ b/server/routes/ws.js @@ -3,12 +3,11 @@ const storage = require('../storage'); const config = require('../config'); const mozlog = require('../log'); const Limiter = require('../limiter'); -const wsStream = require('websocket-stream/stream'); const fxa = require('../fxa'); const { statUploadEvent } = require('../amplitude'); const { encryptedSize } = require('../../app/utils'); -const { Duplex } = require('stream'); +const { Transform } = require('stream'); const log = mozlog('send.upload'); @@ -76,25 +75,19 @@ module.exports = function(ws, req) { }) ); const limiter = new Limiter(encryptedSize(maxFileSize)); - const flowControl = new Duplex({ - read() { - ws.resume(); - }, - write(chunk, encoding, callback) { + const eof = new Transform({ + transform: function(chunk, encoding, callback) { if (chunk.length === 1 && chunk[0] === 0) { this.push(null); } else { - if (!this.push(chunk)) { - ws.pause(); - } + this.push(chunk); } callback(); } }); + const wsStream = ws.constructor.createWebSocketStream(ws); - fileStream = wsStream(ws, { binary: true }) - .pipe(flowControl) - .pipe(limiter); // limiter needs to be the last in the chain + fileStream = wsStream.pipe(eof).pipe(limiter); // limiter needs to be the last in the chain await storage.set(newId, fileStream, meta, timeLimit); @@ -126,8 +119,8 @@ module.exports = function(ws, req) { error: e === 'limit' ? 413 : 500 }) ); - ws.close(); } } + ws.close(); }); }; diff --git a/test/testServer.js b/test/testServer.js index 66b1d46d..235cd427 100644 --- a/test/testServer.js +++ b/test/testServer.js @@ -6,7 +6,7 @@ module.exports = { const webpack = require('webpack'); const middleware = require('webpack-dev-middleware'); const express = require('express'); - const expressWs = require('express-ws'); + const expressWs = require('@dannycoates/express-ws'); const assets = require('../common/assets'); const routes = require('../server/routes'); const tests = require('./frontend/routes');