formidable icon indicating copy to clipboard operation
formidable copied to clipboard

MultipartParser does not call callback in `_flush` if `state == STATE.END`

Open stevenwdv opened this issue 1 year ago • 2 comments

Support plan

  • Which support plan is this issue covered by? (Community, Sponsor, Enterprise): Community
  • Currently blocking your project/work? (yes/no): no
  • Affecting a production system? (yes/no): no

Context

  • Node.js version:
  • Release Line of Formidable (Legacy, Current, Next): Current
  • Formidable exact version: 2.0.5
  • Environment (node, browser, native, OS): node
  • Used with (popular names of modules): -

What are you trying to achieve or the steps to reproduce?

I'm trying to read a MIME buffer and consume partData buffers.

import {Readable} from 'node:stream';
import formidable from 'formidable';

const mime = `--_
Content-Disposition: form-data; name="myname"

myval
--_--
`.replaceAll('\n', '\r\n');
const parser = new formidable.MultipartParser();
parser.initWithBoundary('_');
for await (const {name, buffer, start, end} of Readable.from(mime).pipe(parser))
	console.log(name, buffer ? buffer.subarray(start, end).toString() : '');
console.log('DONE');

CommonJS alternative:

const {Readable} = require('node:stream');
const {pipeline} = require('node:stream/promises');
const {MultipartParser} = require('formidable');

const mime = `--_
Content-Disposition: form-data; name="myname"

myval
--_--
`.replaceAll('\n', '\r\n');
const parser = new MultipartParser();
parser.initWithBoundary('_');
pipeline(Readable.from(mime), parser).then(() => {
	let obj;
	while ((obj = parser.read())) {
		const {name, buffer, start, end} = obj;
		console.log(name, buffer ? buffer.subarray(start, end).toString() : '');
	}
	console.log('DONE');
});

What was the result you got?

partBegin 
headerField Content-Disposition     
headerValue form-data; name="myname"
headerEnd
headersEnd
partData myval
partEnd
end

<exit code 13>

CommonJS:

<no output>

<exit code 0>

Note how 'DONE' is missing. (And for the CJS variant the .then callback is never called.) Exit code 13 means Node.js detected a deadlock and exited: https://stackoverflow.com/q/64602114. This is caused by MultipartParser not calling the done() callback in _flush if this.state === STATE.END. See Multipart.js:78.

What result did you expect?

partBegin
headerField Content-Disposition     
headerValue form-data; name="myname"
headerEnd
headersEnd
partData myval
partEnd
end
DONE

<exit code 0>

Fix

Add else done(); to MultipartParser#_flush.

stevenwdv avatar Aug 08 '22 16:08 stevenwdv

Hmm, actually, even with the fix it still hangs on invalid input like '--_\r\n--_--\r\n'. Not sure how to handle that.

stevenwdv avatar Aug 08 '22 16:08 stevenwdv

Thanks I'll have a look

GrosSacASac avatar Aug 08 '22 19:08 GrosSacASac

Use try catch for the failing example

import {Readable} from 'node:stream';
import {MultipartParser} from '../src/index.js';

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) {
	console.error('error');
	console.error(e);

}
console.log('DONE');

GrosSacASac avatar Jul 05 '23 18:07 GrosSacASac

Also good to know that Streams are now valid async iterables. First time I see for await of loop used like that.

GrosSacASac avatar Jul 05 '23 18:07 GrosSacASac