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

Support multipart/form-data #36

Open
NetOpWibby opened this issue Oct 5, 2020 · 9 comments
Open

Support multipart/form-data #36

NetOpWibby opened this issue Oct 5, 2020 · 9 comments
Labels
enhancement New feature or request

Comments

@NetOpWibby
Copy link

I think this is what's preventing my API from seeing my uploads.

@evert evert added the enhancement New feature or request label Oct 5, 2020
@NetOpWibby
Copy link
Author

My initial foray into creating a helper function with multiparty and formidable brought up TypeError: req.on is not a function.

@evert
Copy link
Member

evert commented Oct 5, 2020

Makes sense. req.on would be a node request specific function, which curveball attempts to abstract to facilitate things like http/2 push, internal requests and native deployment on things like Lambda.

To get access to a Buffer representing the entire request body, you can use the rawBody function available on a request object.

If this is not sufficient, and you need access to something that's more stream-like, let me know. There's probably something we can do about this, although it might require some changes in core.

I'm going to need this very feature next week, so if you're hitting a wall this will come soon.

@NetOpWibby
Copy link
Author

Oh that's fantastic. Perfect timing I guess! I can definitely work on other things in the meantime.

@evert
Copy link
Member

evert commented Oct 6, 2020

@NetOperatorWibby

Looking into this a bit. I first thought I would just add something to this package and just put everything in ctx.request.body.

However, the implication kind of is that this is all going to be stored in memory. That might be ok for our cases (I don't think we'll exceed a few megabytes) but it might not be something you'd want to do, and maybe not a pattern we'd want to encourage.

So I'm thinking now to just suggest people use this package:

https://www.npmjs.com/package/busboy

Which really just means right now you need some way to get access to the underlying stream. So, I'll make that my focus and not add something by default to bodyparser just yet.

For our usecase we'll also use this stream and busbuy to extract the attachments.

@evert
Copy link
Member

evert commented Oct 6, 2020

Actually, we have this already:

ctx.request.getStream().

So I think this should work:

const busboy = new Busboy({ headers: ctx.request.headers.getAll()});
// hook up various events
busboy.pipe(ctx.request.getStream());

@NetOpWibby
Copy link
Author

I'll give it a try!

@NetOpWibby
Copy link
Author

Alright, here's the relevant code I have in my copy of curveball's bodyparser:

parseFormData comes from this issue comment.

function parse(ctx: Context): Promise<void> {
  if (ctx.request.is("multipart/form-data"))
    return parseForm(ctx);

  if (ctx.request.is("json"))
    return parseJson(ctx);

  if (ctx.request.is("x-www-form-urlencoded"))
    return parseUrlEncoded(ctx);

  if (ctx.request.type.startsWith("text/"))
    return parseText(ctx);

  return Promise.resolve();
}

const parseFormData = (event: any) => {
  return new Promise((resolve, reject) => {
    const busboy = new Busboy({
      // headers: {
      //   ...event.headers,
      //   "content-type":
      //     event.headers["Content-Type"] || event.headers["content-type"]
      // }
      headers: event.headers.getAll()
    });

    const result = {
      files: []
    };

    busboy.on("file", (fieldname, file, filename, encoding, mimetype) => {
      console.log(fieldname, file, filename, encoding, mimetype);
      file.on("data", data => {
        result.files.push({
          // @ts-ignore
          contentType: mimetype,
          // @ts-ignore
          file: data,
          // @ts-ignore
          fileName: filename
        });
      });
    });

    busboy.on("field", (fieldname, value) => {
      try {
        // @ts-ignore
        result[fieldname] = JSON.parse(value);
      } catch(err) {
        // @ts-ignore
        result[fieldname] = value;
      }
    });

    // @ts-ignore
    busboy.on("error", error => reject(`Parse error: ${error}`));

    busboy.on("finish", () => {
      event.body = result;
      resolve(event);
    });

    // console.log("+++");
    // console.log(event);
    busboy.write(event.body, event.isBase64Encoded ? "base64" : "binary");
    busboy.end();
  });
};

async function parseForm(ctx: Context) {
  if (!String(ctx.request.headers.getAll()["content-type"]).includes("boundary"))
    return; // This is not a valid multipart request

  ctx.request.body = await parseFormData(ctx.request);
}

The data I'm sending to my API looks like this:

SS 2020-10-06 at 5 06 06 PM

There's a good chance I'm doing something wrong because I'm not seeing busboy write anything. My original code basically copied what's in their README but I deleted that.

@evert
Copy link
Member

evert commented Oct 7, 2020

So I probably wouldn't do this as a middleware, but rather use busbuy directly in the controller where you need it.

In the middleware kind of implies that you parse the whole thing, and then place the result in ctx.request somewhere, but this implies to me that you're storing the entire file in memory.. which feels like a bad idea.

However, if you are going down this route. I'm not seeing where you call pipe() to turn the request stream into busboy and kick off the process. Maybe I'm missing something.

@NetOpWibby
Copy link
Author

You're not missing anything, I just don't know what I'm doing. I've never had to deal with file uploads in any of my projects until now so I'm just slapping things together until something works.

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

No branches or pull requests

2 participants