search

freesoftwarefactory / parse-multipart

A javascript/nodejs multipart/form-data parser which operates on raw data.
MIT License
48 stars 79 forks source link

Lack of filename results in an error #2

Open tmfksoft opened 7 years ago

tmfksoft commented 7 years ago

Although in almost all use cases a filename is supplied, it is not a mandatory value. It'd also be nice if the files "name" parameter is returned.

The error from a lacking filename arg:

/home/thomas/discord_bot/node_modules/parse-multipart/multipart.js:28
                        var k = str.split('=');
                                   ^

TypeError: Cannot read property 'split' of undefined
    at obj (/home/thomas/discord_bot/node_modules/parse-multipart/multipart.js:28:15)
    at process (/home/thomas/discord_bot/node_modules/parse-multipart/multipart.js:37:14)
    at Object.exports.Parse (/home/thomas/discord_bot/node_modules/parse-multipart/multipart.js:87:19)
    at fs.readFile (/home/thomas/discord_bot/scripts/best.js:28:25)
    at FSReqWrap.readFileAfterClose [as oncomplete] (fs.js:445:3)

I hope this isn't too much trouble.

tmfksoft commented 7 years ago

I have written such functionality in, pretty much entirely replacing your process function with my own. It caters for multiple files with the same "name" as well as supporting the lack of Content-Disposition.

Although Content-Disposition is REQUIRED by spec, Amazon AWS returns multipart/related which lacks it. This means my edited code will work without it and merely sets "name" to null.

I can PR this code if you'd like it. However I cannot promise it being bug free. However works fine both with DemoData and Amazon AWS v1 response.

christiansalazar commented 7 years ago

very nice (PR). yes, the process() function should be abstract and provided by a decorator, merely my first intention was to provide a very raw and pure implementation as simple as possible, also keeping distance from other packages providing similar functionality with a lot of dependencies.

i will put my eye on your implementation (due to the content-disposition issue).

be in touch :)

2017-05-24 23:34 GMT-04:00 Thomas Edwards notifications@github.com:

I have written such functionality in, pretty much entirely replacing your process function with my own. It caters for multiple files with the same "name" as well as supporting the lack of Content-Disposition.

Although Content-Disposition is REQUIRED by spec, Amazon AWS returns multipart/related which lacks it. This means my edited code will work without it and merely sets "name" to null.

I can PR this code if you'd like it. However I cannot promise it being bug free. However works fine both with DemoData and Amazon AWS v1 response.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/freesoftwarefactory/parse-multipart/issues/2#issuecomment-303913440, or mute the thread https://github.com/notifications/unsubscribe-auth/ABk4g_3xJ8mQ8RRd9sAOPh1sPNPJ_iEIks5r9PbWgaJpZM4Nl5QL .

--

Cristian A. Salazar

Building Next Generation Apps with Serverless/Microservices, follow me: @AmazonAwsVideos

(+569) 9649-3840 | https://plus.google.com/+ChristianSalazar Santiago. Chile.

Bitbucket https://bitbucket.org/christiansalazarh/ | Github https://github.com/freesoftwarefactory | Stackoverflow http://stackoverflow.com/users/937815/christian | Google + https://plus.google.com/+ChristianSalazar | Blog http://trucosdeprogramacionmovil.blogspot.cl/ | Youtube http://www.youtube.com/c/ChristianSalazar | Npm (NodeJs) https://www.npmjs.com/~csalazar

christiansalazar commented 7 years ago

also, take a look to this resource: https://github.com/freesoftwarefactory/aws-launchpad built by me. It lets you run your lambda functions using a local http gateway similar to apigateway, so in that way you can run your projects 100% local prior to moving to aws.

this https://www.propertywarehouses.com project was built 100% serverless using this platform (link) and runs 100% locally too..

2017-05-24 23:55 GMT-04:00 Cristian Salazar christiansalazarh@gmail.com:

very nice (PR). yes, the process() function should be abstract and provided by a decorator, merely my first intention was to provide a very raw and pure implementation as simple as possible, also keeping distance from other packages providing similar functionality with a lot of dependencies.

i will put my eye on your implementation (due to the content-disposition issue).

be in touch :)

2017-05-24 23:34 GMT-04:00 Thomas Edwards notifications@github.com:

I have written such functionality in, pretty much entirely replacing your process function with my own. It caters for multiple files with the same "name" as well as supporting the lack of Content-Disposition.

Although Content-Disposition is REQUIRED by spec, Amazon AWS returns multipart/related which lacks it. This means my edited code will work without it and merely sets "name" to null.

I can PR this code if you'd like it. However I cannot promise it being bug free. However works fine both with DemoData and Amazon AWS v1 response.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/freesoftwarefactory/parse-multipart/issues/2#issuecomment-303913440, or mute the thread https://github.com/notifications/unsubscribe-auth/ABk4g_3xJ8mQ8RRd9sAOPh1sPNPJ_iEIks5r9PbWgaJpZM4Nl5QL .

--

Cristian A. Salazar

Building Next Generation Apps with Serverless/Microservices, follow me: @AmazonAwsVideos

(+569) 9649-3840 <+56%209%209649%203840> | https://plus.google.com/+ ChristianSalazar Santiago. Chile.

Bitbucket https://bitbucket.org/christiansalazarh/ | Github https://github.com/freesoftwarefactory | Stackoverflow http://stackoverflow.com/users/937815/christian | Google + https://plus.google.com/+ChristianSalazar | Blog http://trucosdeprogramacionmovil.blogspot.cl/ | Youtube http://www.youtube.com/c/ChristianSalazar | Npm (NodeJs) https://www.npmjs.com/~csalazar

--

Cristian A. Salazar

Building Next Generation Apps with Serverless/Microservices, follow me: @AmazonAwsVideos

(+569) 9649-3840 | https://plus.google.com/+ChristianSalazar Santiago. Chile.

Bitbucket https://bitbucket.org/christiansalazarh/ | Github https://github.com/freesoftwarefactory | Stackoverflow http://stackoverflow.com/users/937815/christian | Google + https://plus.google.com/+ChristianSalazar | Blog http://trucosdeprogramacionmovil.blogspot.cl/ | Youtube http://www.youtube.com/c/ChristianSalazar | Npm (NodeJs) https://www.npmjs.com/~csalazar

tsherdiwala commented 7 years ago

@tmfksoft I had been facing the same issue. Thanks for raising the same.

@christiansalazar Any plans on when to accept the PR?

christiansalazar commented 7 years ago

@tmfksoft Hi, because time compromised in my side (2 dev jobs :) ) then it has taken a little bit more time for to give you a nice time for yout PR. Now, because i have 2 pending issues in this repo then i will work on it tomorrow

christiansalazar commented 6 years ago

@tmfksoft Hi there, your PR is not forgotten...:) it is only i have no opportunity to test is further in order to be incorporated to serve the very basic feature AND your extension. Will do in the first oportunity..