Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON array directly in request body is interpreted as Object #850

Closed
Akxe opened this issue Apr 23, 2022 · 10 comments
Closed

JSON array directly in request body is interpreted as Object #850

Akxe opened this issue Apr 23, 2022 · 10 comments
Labels

Comments

@Akxe
Copy link
Collaborator

Akxe commented Apr 23, 2022

Support plan

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

Context

  • Node.js version: 16, but probably all
  • Release Line of Formidable (Legacy, Current, Next): v2
  • Formidable exact version: 2.0.1
  • Environment (node, browser, native, OS): node
  • Used with (popular names of modules): express

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

function formidableMiddleWare(options = {}) {
	return (req, _res, next) => {
		const form = formidable(options);

		form.parse(req, (err, fields, files) => {
			if (err) {
				logger.error('Parsing of form', options, err);

				next(err);
				return;
			}

			req.body = fields
			next();
		});
	};
};
app.post(
  '/form',
  formidableMiddleWare({
    // Do not accept any file by default!
    filter: () => false,
  }),
  (req, res) => {
    res.json({
      req,
      isArray: Array.isArray(req),
    });
  },
);

What was the result you got?

The result of form.parse is an Object not Array

What result did you expect?

The result of form.parse should be an Array.

@Akxe Akxe added the bug label Apr 23, 2022
@GrosSacASac
Copy link
Contributor

Try again with version 3

@Akxe
Copy link
Collaborator Author

Akxe commented Apr 28, 2022

I cannot use v3:

C:\Users\akxe\Documents\Programming\rita\node_modules\@nrwl\node\src\executors\node\node-with-require-overrides.js:16
        return originalLoader.apply(this, arguments);
                              ^
Error [ERR_REQUIRE_ESM]: require() of ES Module C:\Users\akxe\Documents\Programming\rita\node_modules\formidable\src\index.js from C:\Users\akxe\Documents\Programming\rita\dist\apps\server\main.js not supported.

I use

  • Node: 16.13
  • ExpressJS: 4.17
  • @nrwl/node: 13.9 (For compiling, it uses Webpack 5 behind the scene)

@GrosSacASac
Copy link
Contributor

Don't use webpack

@Akxe
Copy link
Collaborator Author

Akxe commented Apr 28, 2022

How is this a solution to this problem?

@tunnckoCore
Copy link
Member

tunnckoCore commented Jun 10, 2022

@Akxe use v3, and probably something like that

https://github.com/tunnckoCore/opensource/blob/75e2766eea27ca4bd01b7e10b3760aecccee1600/%40tunnckocore/execa/src/main.js#L9-L19

with esm to load the formidable v3 esm.

const req = require('esm')(module);
const formidable = req('formidable');

@tunnckoCore
Copy link
Member

tunnckoCore commented Jun 10, 2022

What was the result you got?

The result of form.parse is an Object not Array

Aside from the other comment with possible temporary solution.

Well, i think it's because when there's only one it's an json, when there are multiple it's an array. I don't like that change in behaviors too, but.. i don't like single item arrays either.


uh, wait a sec, why is that isArray: Array.isArray(req), ?

aaand, what you mean with that title, i still don't get it. Is fields an array before you assigning it to the req.body? and what you mean with that it's interpreted as object

@Akxe
Copy link
Collaborator Author

Akxe commented Jun 10, 2022

@tunnckoCore It was meant to be req.body. The problem is not isolated to single-element arrays. It is because the result is by default an Object and properties are added to it as they come. The fix would be to check if the body has only numeric values or to actually check the first character of the body.

@tunnckoCore
Copy link
Member

@Akxe well, i'm really struggling to understand all that.

I looked at the code and yea, fields is always an object, what it should be? Why it shouldn't be?
I don't get what the problem is at all. In parse it comes as object, then you assign it to req.body, then it's an object and not an array which is normal. And what?

@Akxe
Copy link
Collaborator Author

Akxe commented Jun 10, 2022

Hey, the point is that if you pass an array to body of normal requests the BodyParser will give an array in the NodeJS part, but if you pass the same array to the FormData, it will be transformed to an object. It actually fought me off guard, since I had to switch from body request to formData (because I wanted to send a file); The array function I expected to be present was suddenly not present even though there was no indication that it should not.

TLDR; For consistency's sake, it should mimic BodyParser/JSON.stringify as close as possible.

@tunnckoCore
Copy link
Member

can you give actual example and data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants