From 1699ec6fcb6fb01b2f7cacb735fea997358b00a5 Mon Sep 17 00:00:00 2001 From: GrosSacASacs Date: Fri, 25 Aug 2023 16:21:23 +0200 Subject: [PATCH] fix: flush or fail always (don't hang) (#945) * fix: flush or fail always (don't hang) * feat: more descriptive errors * tests: make test work when other tests are run * tests: add test to confirm issue --- CHANGELOG.md | 5 +++ package.json | 2 +- src/parsers/Multipart.js | 42 ++++++++++++------- .../standalone/createDirsFromUploads.test.js | 3 +- test-node/standalone/multipart_parser.test.js | 27 ++++++++++++ 5 files changed, 62 insertions(+), 17 deletions(-) create mode 100644 test-node/standalone/multipart_parser.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cd09f0f..b231847c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +### 3.5.1 + + * fix: ([#945](https://github.com/node-formidable/formidable/pull/945)) multipart parser fix: flush or fail always (don't hang) + + ### 3.5.0 * feature: ([#944](https://github.com/node-formidable/formidable/pull/944)) Dual package: Can be imported as ES module and required as commonjs module diff --git a/package.json b/package.json index 5f258a5c..0561e069 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "formidable", - "version": "3.5.0", + "version": "3.5.1", "license": "MIT", "description": "A node.js module for parsing form data, especially file uploads.", "homepage": "https://github.com/node-formidable/formidable", diff --git a/src/parsers/Multipart.js b/src/parsers/Multipart.js index 8267a013..9d7e4a55 100644 --- a/src/parsers/Multipart.js +++ b/src/parsers/Multipart.js @@ -59,6 +59,14 @@ class MultipartParser extends Transform { this.flags = 0; } + _endUnexpected() { + return new FormidableError( + `MultipartParser.end(): stream ended unexpectedly: ${this.explain()}`, + errors.malformedMultipart, + 400, + ); + } + _flush(done) { if ( (this.state === STATE.HEADER_FIELD_START && this.index === 0) || @@ -68,13 +76,9 @@ class MultipartParser extends Transform { this._handleCallback('end'); done(); } else if (this.state !== STATE.END) { - done( - new FormidableError( - `MultipartParser.end(): stream ended unexpectedly: ${this.explain()}`, - errors.malformedMultipart, - 400, - ), - ); + done(this._endUnexpected()); + } else { + done(); } } @@ -136,7 +140,8 @@ class MultipartParser extends Transform { c = buffer[i]; switch (state) { case STATE.PARSER_UNINITIALIZED: - return i; + done(this._endUnexpected()); + return; case STATE.START: index = 0; state = STATE.START_BOUNDARY; @@ -145,7 +150,8 @@ class MultipartParser extends Transform { if (c === HYPHEN) { flags |= FBOUNDARY.LAST_BOUNDARY; } else if (c !== CR) { - return i; + done(this._endUnexpected()); + return; } index++; break; @@ -159,7 +165,8 @@ class MultipartParser extends Transform { this._handleCallback('partBegin'); state = STATE.HEADER_FIELD_START; } else { - return i; + done(this._endUnexpected()); + return; } break; } @@ -190,7 +197,8 @@ class MultipartParser extends Transform { if (c === COLON) { if (index === 1) { // empty header field - return i; + done(this._endUnexpected()); + return; } dataCallback('headerField', true); state = STATE.HEADER_VALUE_START; @@ -199,7 +207,8 @@ class MultipartParser extends Transform { cl = lower(c); if (cl < A || cl > Z) { - return i; + done(this._endUnexpected()); + return; } break; case STATE.HEADER_VALUE_START: @@ -218,13 +227,15 @@ class MultipartParser extends Transform { break; case STATE.HEADER_VALUE_ALMOST_DONE: if (c !== LF) { - return i; + done(this._endUnexpected()); +return; } state = STATE.HEADER_FIELD_START; break; case STATE.HEADERS_ALMOST_DONE: if (c !== LF) { - return i; + done(this._endUnexpected()); + return; } this._handleCallback('headersEnd'); @@ -311,7 +322,8 @@ class MultipartParser extends Transform { case STATE.END: break; default: - return i; + done(this._endUnexpected()); + return; } } diff --git a/test-node/standalone/createDirsFromUploads.test.js b/test-node/standalone/createDirsFromUploads.test.js index 3e617a29..400dbbfb 100644 --- a/test-node/standalone/createDirsFromUploads.test.js +++ b/test-node/standalone/createDirsFromUploads.test.js @@ -52,7 +52,8 @@ d\r }, body }).then(res => { - deepEqual(fs.readdirSync(uploads), ['x']); + //may also contain tests from other tests + deepEqual(fs.readdirSync(uploads).includes('x'), true); deepEqual(fs.readdirSync(`${uploads}/x`), ['y']); deepEqual(fs.readdirSync(`${uploads}/x/y`), ['z.txt']); strictEqual(res.status, 200); diff --git a/test-node/standalone/multipart_parser.test.js b/test-node/standalone/multipart_parser.test.js new file mode 100644 index 00000000..df56b7c4 --- /dev/null +++ b/test-node/standalone/multipart_parser.test.js @@ -0,0 +1,27 @@ +import {Readable} from 'node:stream'; +import MultipartParser from '../../src/parsers/Multipart.js'; +import {malformedMultipart} from '../../src/FormidableError.js'; + +import test from 'node:test'; +import assert, { deepEqual } from 'node:assert'; + + + +test('MultipartParser does not hang', async (t) => { + const mime = `--_\r\n--_--\r\n`; + const parser = new MultipartParser(); + parser.initWithBoundary('_'); + try { + for await (const {name, buffer, start, end} of Readable.from(mime).pipe(parser)) { + console.log(name, buffer ? buffer.subarray(start, end).toString() : ''); + } + + } catch (e) { + deepEqual(e.code, malformedMultipart) + return; + // console.error('error'); + // console.error(e); + + } + assert(false, 'should catch error'); +}); \ No newline at end of file