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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support binary option for multipart/form-data type #130

Closed
2 tasks done
6unpk opened this issue Oct 5, 2022 · 7 comments
Closed
2 tasks done

Support binary option for multipart/form-data type #130

6unpk opened this issue Oct 5, 2022 · 7 comments

Comments

@6unpk
Copy link

6unpk commented Oct 5, 2022

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the feature has not already been requested

馃殌 Feature Proposal

Hello! I'm using this module with @fastify/multipart. recently i found that when request type is multipart/form-data, it must be decoded with binary options like Buffer.from(data, 'binary') But, now it seems this module doesn't support binary options. So could i add binary option?

Motivation

When i send this request

curl --location --request POST 'http://localhost:3000/upload/templates' \
--form 'thumbnail=@"/Users/junwoopark/Downloads/24939410.png"' \
--form 'template="{}"' \
--form 'ratio="1"' \
--form 'categories=""'

lambda handler is (test with serverless-offline)

const ffy = require('fastify');
const fs = require('fs');
const fastify = ffy();

const opts = {
  attachFieldsToBody: true,
};

fastify.register(require('@fastify/multipart'), opts)

fastify.post('/upload/templates', async function (req, reply) {
  const data = await req.body;
  console.log(data);
  fs.writeFileSync('./image.png', data.thumbnail._buf);
  reply.send('done');
})

export const main = awsLambdaFastify(fastify);

buffer result must be

 thumbnail: {
    fieldname: 'thumbnail',
    filename: '24939410.png',
    encoding: '7bit',
    mimetype: 'image/png',
    file: FileStream {
      _readableState: [ReadableState],
      _events: [Object: null prototype],
      _eventsCount: 5,
      _maxListeners: undefined,
      bytesRead: 5062,
      truncated: false,
      _read: [Function (anonymous)],
      [Symbol(kCapture)]: false
    },
    fields: [Circular *1],
    _buf: <Buffer 89 50 4e 47 0d 0a 1a 0a 00 00 00 0d 49 48 44 52 00 00 00 c8 00 00 00 c8 08 02 00 00 00 22 3a 39 c9 00 00 13 8d 49 44 41 54 78 9c ec dd 79 50 53 e7 fe ... 5012 more bytes>,
    toBuffer: [AsyncFunction: toBuffer]
  },

but actual result of this version is

thumbnail: {
    fieldname: 'thumbnail',
    filename: '24939410.png',
    encoding: '7bit',
    mimetype: 'image/png',
    file: FileStream {
      _readableState: [ReadableState],
      _events: [Object: null prototype],
      _eventsCount: 5,
      _maxListeners: undefined,
      bytesRead: 7603,
      truncated: false,
      _read: [Function (anonymous)],
      [Symbol(kCapture)]: false
    },
    fields: [Circular *1],
    _buf: <Buffer c2 89 50 4e 47 0d 0a 1a 0a 00 00 00 0d 49 48 44 52 00 00 00 c3 88 00 00 00 c3 88 08 02 00 00 00 22 3a 39 c3 89 00 00 13 c2 8d 49 44 41 54 78 c2 9c c3 ... 7553 more bytes>,
    toBuffer: [AsyncFunction: toBuffer]
  },

Example

I think new option enforceBinaryOption should be added

property description default value
enforceBinaryOption false
module.exports = () => {
    options.enforceBinaryOption = options.enforceBinaryOption ?? false
     ...

    //  multipart/form-data only works if encoding is binary 
    // const payload = Buffer.from(event.body, 'binary')
    // const payload = Buffer.from(event.body, event.isBase64Encoded ? 'base64' : 'utf8')
   const payload = Buffer.from(event.body, options.enforceBinaryOption ?  'binary' :  (event.isBase64Encoded ? 'base64' : 'utf8'))
}
@6unpk 6unpk mentioned this issue Oct 5, 2022
4 tasks
@adrai
Copy link
Member

adrai commented Oct 5, 2022

Would using the binaryMimeTypes option also work instead of introducing a new option?
What if you have other routes not needing binary?

@adrai
Copy link
Member

adrai commented Oct 5, 2022

btw: have you defined multipart/form-data as a binary media type for your API, like described here?
If so can you share how your lambda event looks like?

@6unpk
Copy link
Author

6unpk commented Oct 5, 2022

umm.. i tested with serverless-offline plugin and serveless config(serverless.ts) looks like

provider: {
    name: 'aws',
    runtime: 'nodejs14.x',
    lambdaHashingVersion: 20201221,
    stage: 'dev',
    region: 'ap-northeast-2',
    logRetentionInDays: 14,
    memorySize: 256,
    apiGateway: {
      binaryMediaTypes: ['multipart/form-data', 'image/*'],
      apiKeySourceType: 'HEADER',
      minimumCompressionSize: 5000,
    },
 ...
}

and this is lambda event

'create-template': {
      handler: 'src/functions/create-template/handler.main',
      events: [
        {
          http: {
            path: 'upload/templates',
            method: 'post',
            cors: corsPolicy,
            private: true,
          },
        },
      ],
    },

Would using the binaryMimeTypes option also work instead of introducing a new option? What if you have other routes not needing binary?

i already tried to solve this problem with binaryMimeType. but i can't still find reason why not working with binaryMimeType...

@adrai
Copy link
Member

adrai commented Oct 5, 2022

and this is lambda event

I mean the real aws lambda event that is passed in your lambda handle function...

something like:

exports.handler = async (event, context) => {
  console.log(JSON.stringify(event, null, 2))
  return proxy(event, context)
}

@adrai
Copy link
Member

adrai commented Oct 5, 2022

fyi: I just added some multipart tests.... seems to work: e274f7e

@6unpk
Copy link
Author

6unpk commented Oct 6, 2022

oh,, i'm sry for confusion. atfter i have tried many times, figured out it happens due to local environment. it works on real aws api gw and lambda environment. thanks for reply!

@6unpk 6unpk closed this as completed Oct 6, 2022
@6unpk
Copy link
Author

6unpk commented Oct 6, 2022

close PR too

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

No branches or pull requests

2 participants