Cancel/abort upload process (Feature request)

[xarguments]

When the lib receives files and fields, it starts to receive (upload) files in any case.
For example, I check form fields for emptiness, and reject the request. But it still accepts the files and stores them, and I can't prevent it.

form.on("field", function (name, value) {
  // Reject if a form field value is empty
  if (_.isEmpty(value)) {
    return res.sendStatus(400);
    // Want to cancel processing of the form here, but the lib continues
  }
});
form.on("file", function (name, value) {
  // And accepts the file, stores it, and reaches here
});

So in this example, if the received form consists of an empty field, and a file, its not possible to stop the file from being accepted (correct me if I've mistaken).

[tunnckoCore]

Absolutely. That's a thing that i was thinking for a long time and received few issues related to that. Such as https://github.com/tunnckoCore/koa-better-body/issues/65 - package that is my main source for finding bugs (and used heavily for years by many) and related things to Formidable. We had discussions for a lot of the things.

The linked issue is for file size limitting, but if there was way for canceling it wouldn't exist probably. So from here follows... throw from the event handler? - may work for now, until we implement abort/cancel.

edit: oh, this comment was probably better for the #456 and #457, i'll link it there.

[hassan-jahan]

Any news?

[tunnckoCore]

@hassan-jahan, unfortunately, don't think so.

[GrosSacASac]

I am on it

[noonii]

any news @GrosSacASac ? don't want to be forced to switch libs tbh

[tunnckoCore]

@noonii, doesn't that

So from here follows... throw from the event handler? - may work for now, until we implement abort/cancel.

Work? Throw from the handler (attaching any useful metadata to the error) then from the error listener/handler one you can cleanup and exit?

[noonii]

@tunnckoCore

I've tried to force quit using form._error(new Error('error message')); and this.emit('error') and even pausing the request before it all; however, the file continues to upload.

I know where it's uploading and I can customize onPart to stop the upload but my point is that I'm basically rewriting far too much code than necessary.

The only time I've seen the parser successfully destroy uploading is here .

I've been adding guards in fileBegin to destroy likewise to no avail.

EDIT: I ended up overriding form.onPart and including my guards there to halt the uploads.

EDIT2: If multiples is set to false, the second file is still uploaded but only 1 file is returned in the response by default.

[tunnckoCore]

@noonii thanks for the feedback. That's the reason of form.onPart existence, to be able to override it somehow if needed.

If multiples is set to false, the second file is still uploaded but only 1 file is returned in the response by default.

Interesting. I have some thoughts. Can you bring some code (your form.onPart)?

[noonii]

@tunnckoCore

form.onPart = function(part) {
   if (part.filename !== undefined) {
      // Validate allowed file ext
      if (!isSupportedFileExt(part.filename)) { // video or caption file
          this.emit('error', new Error(`File extension for [ ${part.filename} ] is not supported.`));
          return;
      }
      // Guard multiple files
      // Guard file size
   }
   // formidable default form handling
   form.handlePart(part);
}

export const isSupportedFileExt = (file: string | File): boolean => {
    const filename: string = typeof file === 'string' ? file : file.name
    const ext: string = filename.split('.').pop(); // non-regex solution
    return ['mp4', 'm4v', 'vtt'].includes(ext);
}

This implementation guards against specified files, but will upload otherwise.

[tunnckoCore]

@noonii Thanks. Okay, another approach... what about putting it on fileBegin handler which receives part.name, file, so you can later file.emit('error') which in turn will call ... hmm. You probably tried that.

Okay. This one?

// in future we should just pass the whole `part` here
// instead of `part.name`
form.on('fileBegin', (partName, file) => {
  if (file.name) {
      // Validate allowed file ext
      if (!isSupportedFileExt(part.filename)) { // video or caption file
          // and in the end (before emitting/throwing)
          file.open = () => {}
          file.write = () => {}
          file.end = () => {}
          this.emit('error', new Error(`File extension for [ ${part.filename} ] is not supported.`));
          return;
      }
      // Guard multiple files
      // Guard file size

      // and in the end (before emitting/throwing)
      file.open = () => {}
      file.write = () => {}
      file.end = () => {}
   }

})

The cool thing of references, haha...

edit: part.filename effectively becomes file.name from inside the File constructor.

[tunnckoCore]

Oooor... again with file event

form.on('file', (partName, file) => {
 // when needed
  file._writeStream.destroy();
  this.emit('error')
  // or 
  // file.emit('error')
})

but I guess it will make a file again... not sure.

[tunnckoCore]

I know where it's uploading and I can customize onPart to stop the upload but my point is that I'm basically rewriting far too much code than necessary.

True. But we'll figure it out : ) Feedback and trying is the key ;p I'd love to be in v2. That's probably why I don't releasing it yet. There are such very important & significant problems.

[noonii]

Oooor... again with file event

form.on('file', (partName, file) => {
 // when needed
  file._writeStream.destroy();
  this.emit('error')
  // or 
  // file.emit('error')
})

but I guess it will make a file again... not sure.

While the file event would be an alternative, we would overwrite the guards against multiples. And we don't want to rewrite it all.

[tunnckoCore]

Yup. This multiples thing sucks, never liked it though 😆

[tunnckoCore]

Okay, I think the easiest and cleanest approach for now is the "file methods noop-ing" (file.open = () => {} & etc), so it woundn't even create a writeStream in first place.

Other future approach could Validators API, like options.validators = [] and then calling them from the part.on('data') passed with the this, part, file and the buffer. ry catch inside that loop, and if catch error call this._error. Plus some if check before the file.open() couple lines above.

[GrosSacASac]

Try the branch https://github.com/node-formidable/formidable/tree/filter-upload

let customError = false;
const form = formidable({
    filter: function ({name, originalFilename, mimetype}) {
        return !customError;
    }
});

form.on("field", function (name, value) {
    // Reject if a form field value is empty
    if (_.isEmpty(value)) {
        customError = true
        return;
    }
});

form.parse(req, (err, fields, files) => {
    if (err || customError) {
        res.sendStatus(400);
        return;
    }
    // ...
});