Using form.onPart() to validate file type/name and then form.parse to receive the file

[droddy]

Should I be able to use form.onPart() along with form.parse() in order to

form.onPart(): test for valid file types and return validation message if that fails or continue on to...

form.parse(): parse the file if the validation in form.onPart() passes?

I can't seem to find the right way to do this.

CONTROLLER:

class {
  public initializeRoutes() {
    this.router.post(
      '/file',
      this.uploadFile,
    );
  }
  formidableOptions = {
    encoding: 'utf-8',
    keepExtensions: true,
    // 3 mb for news image and attachments. override otherwise
    maxFileSize: 3 * 1024 * 1024,
  };

  uploadFile = (req: Request, res: Response) => {
    const typeValidationMessage = 'Please choose a JPEG, GIF, PDF or DOC file.';

    const form = formidable(this.formidableOptions);

    form.onPart = (part: { filename: any; mime: any }) => {
      let isValid = true;
      if (part.filename || part.mime) {
        if (part.filename) {
          isValid =
            isValid && !AttachmentValidator.isInvalidFileName(part.filename);
        }

        if (part.mime) {
          isValid =
            isValid && !AttachmentValidator.isInvalidFileType(part.mime);
        }
        if (!isValid) {
          return res.status(400).json({ message: typeValidationMessage });
        }
      }
    };

    try {
      UploadService.upload(
        req,
        // onSuccess callback
        async (fileLocation: string, originalName: string) => {
          try {
            await C3DistributionService.sendAttachment(
              fileLocation,
              originalName,
            );
          } catch (error) {
            this.logger.error(error);
            return res.status(500).json({ message: error.message });
          } finally {
            // try catch swallow in case only the delete tmp file (unlinkSync) fails
            try {
              fs.unlinkSync(fileLocation);
            } catch (error) {
              this.logger.error(error);
            }
          }
          return res.status(201).send();
        },
        // onFail callback
        () => {
          return res.status(500).json({
            message: 'unexpected error from upload service callback run',
          });
        },
      );
    } catch (error) {
      this.logger.error(error);
      return res.status(500).json({ message: error.message });
    }
  };
}

SERVICE/formidable parse wrapper:

const formidableOptions = {
  encoding: 'utf-8',
  uploadDir: UPLOAD_DIR,
  keepExtensions: true,
  // 3 mb for news image and attachments. override otherwise
  maxFileSize: 3 * 1024 * 1024,
};

const UploadService = {
  upload: (req: Request, onSuccess?: Function, onFail?: Function): void => {
    ensureTmpFilePath();

    const form = formidable(formidableOptions);

    form.parse(req, (err: any, fields: any, files: { files: File }) => {
      if (err) {
        logger.error('formidable err: %j', err);
        if (onFail) {
          onFail();
        }
      } else if (onSuccess) {
        onSuccess(files.files.path, files.files.name);
      }
    });
  },
};

Originally posted by @droddy in #387 (comment)
EDIT: SYNTAX highlighting

[auto-comment]

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]

True. And not. Just realized that we don't document the usage of onPart, or at least the drawbacks that occur when you override it. This is the most important piece of the whole formidable. And there are few things to be noted, cuz by overriding it you remove 1) the whole writing to disk, 2) fields handling (when not files) and 3) the emitting (fileBegin) and I suppose that's when your problem starts cuz the parser can't work properly, 4) size handling.

The easiest thing would be (and I guess should be recommended) is calling the _handlePart manually from your onPart. Maybe try adding this._handlePart(part) after your last if in your onPart.

I will look into it more later.

[droddy]

Thanks! I'll give it a shot.

I think I'm about to try to become a contributor BTW! I think I could help improve documentation for a start.

[droddy]

I ended up going with express-fileupload (busboy wrapper) because the middleware implementation allows me to respond with a 413 response as early as possible in the request/response cycle. I believe that's before the stream is parsed at all.

[tunnckoCore]

I think I could help improve documentation for a start.

That would be great, yea. :)

I believe that's before the stream is parsed at all.

True. Most probably. Busboy is great, for sure! Here we are kind of stopped because of legacy and architecture.

I'm thinking to release v2 then a whole rewrite could happen.

[FlorentinTh]

@droddy multer let you do this also, you have a nice fileFilter function that let you control each file before they are uploaded. Also if you want to check more seriously the type of file users of yours are uploading I suggest using multer configured with an in memory storage, then in the callback that can be directly set in your middleware, you can verify the magic number of files thanks to a package like file-type and if they satisfy your requirements, write them to the disk simply through fs.writeFile since you have access to the buffer of the file inside multer's callback.

@tunnckoCore integrate all of this in a nice v2 of formidable should be pretty formidable 😊

PS: I'm not promoting anything here, just give you some options to complete what you want.

[tunnckoCore]

Yup 😆 Thought so, but... seems like I even struggle to finally cut v2, haha.

These features are most wanted, even back then (2016-ish) when I wasn't core maintainer here and but was maintaining my koa-better-body which is based on formidable. Kind of the same story... I was blocked to release its new major (still) because formidable, haha.

Anyway. 😸

[Alukardz]

@tunnckoCore Hello, first google search on this, got me here, any news on this? Can we use beginFile to do this validation before uploading the file?

[tunnckoCore]

@Alukardz nothing new for the moment sorry, I wasn't active last few months.

[GrosSacASac]

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