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

How not to upload with form.parse() ? #609

Closed
TayPark opened this issue Apr 17, 2020 · 16 comments · Fixed by #666
Closed

How not to upload with form.parse() ? #609

TayPark opened this issue Apr 17, 2020 · 16 comments · Fixed by #666

Comments

@TayPark
Copy link

TayPark commented Apr 17, 2020

Context

  • node version: 12.14.1
  • module (formidable) version: canary
  • environment (e.g. node, browser, native, OS): Nodenv, Chrome, WSL2, Win10 Pro
  • used with (i.e. popular names of modules): express, mysql
  • any other relevant information: -

What problem are you trying to solve?

I have a problem with using formidable, it's a simple but I can't find any site with this problem.

"In case I have a upload webapp with formidable, how not to upload an empty file? It always upload an empty file, title like upload_3f0934128511fd38dha1.... and no content"

I'm using this module like below in expressJS

router.post('/compose', async function (req, res, next) {  
if (req.body.imgFile == undefined ) { // no upload }
else {
    form.parse(req, async (err, fields, files) => {
    // some business logic
    }
}

but it doesn't work anyway. I think form.parse() has no option to not upload.

So if form.parse() in formidable has a function to make webapp upload or not, plz comment below. Or same function with same feature, plz comment

thx

@auto-comment
Copy link

auto-comment bot commented Apr 17, 2020

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
Copy link
Member

tunnckoCore commented Apr 17, 2020

No possible at the moment. It should be in the v2 final. I had some idea about that, I just didn't jump on the code to test it.

@TayPark
Copy link
Author

TayPark commented Apr 22, 2020

Hello again. I have a question. I'm also using form.parse() for passing req, but it doesn't work on this situation

router.post('/update',` async function (req, res, next) {
    console.log(req.body.imgFile); // some imgs, also req is passing here

    if ( some control words ) {
        form.parse(req, async function (err, fields, files) { 
            console.log(files) // nothing here, 
            console.log(req) // nothing here, too
        })
    }
});

Also, when I call form.parse(), images automatically uploaded on server, but it doesn't work anyway. How can I solve this? should I use .on() and .emit() instead?

@tunnckoCore
Copy link
Member

I think the callback to parse cannot be async function, yea, use the event API.

@TayPark
Copy link
Author

TayPark commented Apr 23, 2020

thx a lot XD

@xr0master
Copy link

xr0master commented Aug 5, 2020

I know it is old, but you can overwrite the logic, it's not so hard.
For example, I needed to get the buffer instead of saving the file.

import {IncomingForm, Part} from 'formidable';

export class FormParse extends IncomingForm {

  private handleFilePart(part: Part) {
    let chunks = [];

    part.on('data', (chunk) => {
      chunks.push(chunk);
    });

    part.on('end', () => {
      this.emit('field', part.name, {
        contentType: part.mime,
        filename: part.filename,
        content: Buffer.concat(chunks)
      });
    });
  }

  public onPart(part: Part) {
    if (part.filename === undefined) {
      this.handlePart(part);
    } else {
      this.handleFilePart(part);
    }
  }
}

@leonardovillela
Copy link
Contributor

I will take a look at this issue during the weekend.

@leonardovillela
Copy link
Contributor

@GrosSacASac I think this issue is solved by the new option allowEmptyFiles.

@GrosSacASac
Copy link
Contributor

This issue is about providing a convenient way of not saving the file. The original issue was about empty files only that is true, but we want more now.

@leonardovillela
Copy link
Contributor

leonardovillela commented Dec 13, 2020

It's like an option to avoid the file to be saved? In this case why formidable will be used for? Only for parsing the upload file request?

I'm missing exactly what you want as formidable feature.

@GrosSacASac
Copy link
Contributor

Yes. In this case formidable will be used to parse "multipart/form-data" correctly. Not saving in a file is useful for:

  • Doing something with the file content immediately
  • Server-less environments, where the filesystem is not usable
  • Saving the file in a Database instead

@xr0master
Copy link

xr0master commented Dec 13, 2020

I would even suggest that this should be the default behavior.
It is very rare case to save files on the same machine, most likely you will upload it to the cloud. Otherwise it is very easy to lose all files when the machine stops.

If the library returns the buffer by default (or stream, it can be flagged), then it is quite easy to save the file on the logical machine. But maybe such changes should be made in version 3. It may be worth removing this logic altogether, which will reduce the package size. Whoever needs it, he himself implements it.

@leonardovillela
Copy link
Contributor

@xr0master I think to turn this as default now and introduce a breaking change is not worthy, maybe change the defaults in future major versions.

@xr0master
Copy link

@leonardovillela I said so, didn't I? But on the other hand, even version 2 has not yet been released. Such changes may well be.

@tunnckoCore
Copy link
Member

tunnckoCore commented Dec 15, 2020

It's okay to be in v2. The canary doesn't have any (unless u do something very specific) breaking changes anyway.

@xr0master I would even suggest that this should be the default behavior.

Yep, sounds good. I imagined some super simple and comfortable-looking (to me) API in v3 #635. Some of these surely can be in v2 i think.

A lot of users are using multer because of its "stores" and I guess most are using the "memory store" and later passing it to the cloud. If we have such clean API (as 635) on top of latest Node Streams it will definitely be up to date to the current day and age, and even easier than the other alternatives haha.

@leonardovillela
Copy link
Contributor

I think can be removed as a pinned issue in this repository.

@GrosSacASac GrosSacASac unpinned this issue Dec 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants