fastify / fastify-multipart

Multipart support for Fastify
MIT License
455 stars 99 forks source link

`attachFieldsToBody` makes `request.file()` return undefined. #472

Open Ryooshuu opened 11 months ago

Ryooshuu commented 11 months ago

Prerequisites

Fastify version

4.21.0

Plugin version

7.7.3

Node.js version

20.3.1

Operating system

Windows

Operating system version (i.e. 20.04, 11.3, 10)

10.0.19044 Build 19044

Description

I'm not entirely sure if this is an intentional change, but it's certainly one that I don't personally agree with. As far as I'm aware and have explored, there is no possible way to get the name of the file as request.body.(field where the file should be) provides only the buffer and nothing else.

Steps to Reproduce

import fastify from "fastify";
import "@fastify/multipart";

const instance = fastify({ logger: true });
instance.register(require("@fastify/multipart"), {
    attachFieldsToBody: "keyValues",
});

instance.route({
    method: "POST",
    url: `/files/`,
    handler: async (request, reply) => {       
        const file = await request.file();
        console.log(file?.filename || "no file"); // returns "no file"

        return {
            code: 200,
            message: "ok"
        }
    }
})

instance.listen({ port: 8090 }, (err, address) => {
    if (err) {
        instance.log.error(err);
        process.exit(1);
    }
    instance.log.info(`Server listening on ${address}`);
})

Expected Behavior

Instead of returning no file, it should return the data of the sent file without requiring the usage of request.body.(field where the file should be).

mcollina commented 11 months ago

Would you like to send a PR?

gurgunday commented 11 months ago

If you want the name of the file, you should be using attachFieldsToBody: true

We could make a PR where we pass file fields directly to the body

Ryooshuu commented 11 months ago

I'm using attachFieldsToBody: "keyValues" explicitly to keep the body properties returning only the value given to it during the request. Though, I suppose internally I could make it possible to use attachFieldsToBody: true and make a function that returns the value of that property.

Thank you for the PR, however, I'll be looking forward to it being merged.