Multiple fields with the same name are ignored.

[StephenLynx]

According to the standard, multiple fields with the same name are allowed. But this library just ignores every one after the first one. And no, you don't need [] at the end. Any field with any name should represent an array when present multiple times.

[auto-comment[bot]]

Thank you for raising this issue! We will try and get back to you as soon as possible. Please make sure you format it properly, followed our code of conduct and have given us as much context as possible. /cc @tunnckoCore @GrosSacASac

[tunnckoCore]

Okay. I hate multiples option anyway. We can probably remove it or replace/update it. It's the way for better defaults.

Thanks again will take it as note.

[StephenLynx]

Excuse me if you receive this twice, I replied by e-mail but it isn't showing here.

Might as well make as if it's always enabled. Always return parameters and files in an array. If people dislike it and rely on the old ambiguous behavior, could implement it as an opt-in setting.

[tunnckoCore]

Yup, exactly what I was thinking.

[GrosSacASac]

https://github.com/node-formidable/formidable/issues/485 https://github.com/node-formidable/formidable/issues/292

[tunnckoCore]

@GrosSacASac thanks.

[tunnckoCore]

Actually... You can do whatever you want and handle it yourself, through the events API. It's just not the default. :thinking:

Consider the following

import formidable from 'formidable';

const form = formidable();
const fields = [];
const files = [];

form.on('field', (fieldName, fieldValue) => {
  fields.push({ field: fieldName, value: fieldValue })
});

form.on('file', (fieldName, file) => {
  files.push({ field: fieldName, value: file })
});

form.on('error', console.error);
form.on('end', () => {
  console.log({ fields, files });
});

// we don't use a callback, because to not waste time
// and to not add our default event listeners.
form.parse(req);

@StephenLynx mm?

I'll document all that for sure. :) People must read docs. I don't buy the thing "users don't read docs", they must or otherwise unexpected things can happen and it's not the module's or its author's fault. :)

[StephenLynx]

You know, that IS a valid approach. But if you start relying exclusively on that, then returning the parsed data in the parse's callback without the other fields makes that option kind of crippled.

[tunnckoCore]

then returning the parsed data in the parse's callback without the other fields

Sorry, what you mean with that? You should not mix APIs anyway :thinking:

[StephenLynx]

I mean the form.parse function. That callback returns the fields and files objects. In the fields object it doesn't return the multiple appearances of the parameter.

form.parse(req,function(error, fields, files){
   //what i'm talking about is the fields variable being limited by a single appearance of each 
  //parameter instead of being able to have arrays with all the values put under the same name.
});

edit: fuck's sake formatting stuff here is hard :c

[tunnckoCore]

fuck's sake formatting stuff here is hard :c

Yup, haha.

On the point. You don't need the callback when using the events API - that's the thing, things will still work.

I started thinking for v3 already, haha. When we'll be entirely only streaming api & no writing to the disk, so no such confusions.

Something like...

// old Node Streams and `through2` package
formidable(req[, options]).pipe(through2(({ type, name, value }, enc, cb) => {
  console.log(type); // 'field' or 'file'
  console.log(name); // name attribute of the input
  console.log(value); // value of the input, no matter it's 'field' or 'file'

  cb();
}));

// and of course, because latest Streams API
// is available with `for await ... of` we probably could do
for await (const res of formidable(req)) {
  console.log(res.type, res.name, res.value);
  // or even sweeter api
  res.getFileName()
  res.isFile();
  res.getValue();
  // and so on...
}

edit: By the way, multiparty package isn't returning arrays too.

[StephenLynx]

You are right, you don't need it. My point was that if the parse function is just unable to do the same, then it becomes a bit useless.

And a thing about not writing to the disk, I am not really sure if that's feasible. People can upload really, really, large files. I can see that easily turning into an attack vector. And yes, you do have limits. But sometimes you want to allow those large files by design. If your site allow 100mb files and you have 4gb ram, all it takes is less than 40 requests simultaneously to fill your RAM.

[StephenLynx]

@tunnckoCore I use these options on multiparty 4.2.x: uploadDir : uploadDir autoFiles : true maxFilesSize : maxRequestSize

And every single parameter returns as an array.

https://gitgud.io/LynxChan/LynxChan/-/blob/master/src/be/engine/formOps.js#L442

[StephenLynx]

Also, from their readme on their npm page:

fields is an object where the property names are field names and the values are arrays of field values. files is an object where the property names are field names and the values are arrays of file objects.

So I think you messed up something if you are not getting arrays.

edit: keep in mind they can't support node 14 yet. That being the whole reason I started looking into alternatives.

[tunnckoCore]

My point was that if the parse function is just unable to do the same, then it becomes a bit useless.

It's not. It just provides the different approach. I always believe that things should be flexible, replaceable, extendable, for the different use cases.

And a thing about not writing to the disk, I am not really sure if that's feasible.

Very true. I thought about that too. But again, flexibility. There are a lot of issues that want such feature (for them) (I don't see why they need that instead of just moving the file after upload has finished, but... that's their choice), so... it's up to them to decide. Developer's job is to provide sane defaults for the common sense, and enough flexibility and no lock-ins for the rest of the people and use cases.

So I think you messed up something if you are not getting arrays.

Ah, probably. I just scanned the readme quickly.

[StephenLynx]

Is not really flexible if you can only use one approach to get the result you need. And I'd also raise the issue of only one approach actually respecting the HTTP standard.

[tunnckoCore]

Is not really flexible if you can only use one approach to get the result you need.

Actually, true. Depends on the perspective. It's flexible in terms that there's not only one way to approach things and there's no locking with a single choice, as many things do in Nodejs and that's one of the reasons to have hundreds of packages doing the same thing in different way, which in turn leads to more confusion. If it's in a single package, it still can be confusing, but not that much cuz it can be documented, compared, explained and it's a centralized source of truth. It's harder for the users to compare different packages, by different maintainers.

And I'd also raise the issue of only one approach actually respecting the HTTP standard.

It can be as respecting as the user want it to be, through one of the APIs which just streams the parsed data without any modifications and opinions on our side.

The funny part is that both formidable and multiparty are built by the same person, haha. And busboy is the most unopinionated and smallest, I really love it all these years.

[GrosSacASac]

npm i node-formidable/formidable#3.x