search

freesoftwarefactory / parse-multipart

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

Handle missing filename header #27

Open carlonluca opened 4 years ago

carlonluca commented 4 years ago

Quick fix to avoid crashing when filename is not specified.

christiansalazar commented 4 years ago

thank you very much. in this moment is not possible to me to merge this fix, because this master branch is currently under production and i have no time in this moment to fix and test your work. i appreciate your effor, in the future i will do that. :)

El vie., 1 may. 2020 a las 9:09, Luca Carlon (notifications@github.com) escribió:

Quick fix to avoid crashing when filename is not specified.

You can view, comment on, or merge this pull request online at:

https://github.com/freesoftwarefactory/parse-multipart/pull/27 Commit Summary

  • Handle missing filename header.

File Changes

Patch Links:

— 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/pull/27, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMTRA5DAVHCNZKSKV7OU23RPLC2FANCNFSM4MXCZOSA .

Satwa commented 4 years ago

What you can do then is adding contributors to the repo so that other people can maintain this library for you. PRs from years ago are still waiting to be merged and it doesn't make sense you don't delegate the maintenance if you don't have time.

christiansalazar commented 4 years ago

you are right. i am very sorry about this. will do that. can you help me on this ?

El jue., 7 may. 2020 a las 5:29, Joshua (notifications@github.com) escribió:

What you can do then is adding contributors to the repo so that other people can maintain this library for you. PRs from years ago are still waiting to be merged and it doesn't make sense you don't delegate the maintenance if you don't have time.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/freesoftwarefactory/parse-multipart/pull/27#issuecomment-625140404, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMTRA2OG5LX3BKUKHFLKBDRQJ5QVANCNFSM4MXCZOSA .

Satwa commented 4 years ago

Of course, I can give few hours per month to look at issues and PRs on this package

christiansalazar commented 4 years ago

the problem is, it is not monetized, so i have no resources to put on it (money). i know things are not free.

this solution was made few years ago for a customer on the united states, which dont to put more money on this project, so i have no access to their servers anymore (of course, i neither work for free), if any update affects the master branch and it causes a failure on his project then it will be a problem to me.

(and sorry my broken english).

El jue., 7 may. 2020 a las 12:47, Joshua (notifications@github.com) escribió:

Of course, I can give few hours per month to look at issues and PRs on this package

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/freesoftwarefactory/parse-multipart/pull/27#issuecomment-625370165, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMTRA5L57UDKN4GZUQE5YTRQLQ3DANCNFSM4MXCZOSA .

Satwa commented 4 years ago

but if the test data test passes, this shouldn't break anything? as you prefer, you could maybe at least make a fork of your own project that would be for other developers and keep this for you client

christiansalazar commented 4 years ago

it may be ok on my case, but what if other depends on this master branch and the test case does not cover their own cases. That was one of the main reason behind i did not make any changes to the master branch. Many others forks and implements their own cases and it works nice.

El jue., 7 may. 2020 a las 12:59, Joshua (notifications@github.com) escribió:

but if the test data test passes, this shouldn't break anything? as you prefer, you could maybe at least make a fork of your own project that would be for other developers and keep this for you client

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/freesoftwarefactory/parse-multipart/pull/27#issuecomment-625376796, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMTRAZHI6RMHP3FFHSIYUDRQLSHBANCNFSM4MXCZOSA .

Satwa commented 4 years ago

that's the game of open-source, they'll either make a PR or open an issue if you don't plan on updating the dependency, you should make it clear on repo description and readme imo but don't give hope to others waiting for you to merge

christiansalazar commented 4 years ago

you're right. i will do that very soon (tomorrow or today).

El jue., 7 may. 2020 a las 13:23, Joshua (notifications@github.com) escribió:

that's the game of open-source, they'll either make a PR or open an issue if you don't plan on updating the dependency, you should make it clear on repo description and readme imo but don't give hope to others waiting for you to merge

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/freesoftwarefactory/parse-multipart/pull/27#issuecomment-625388968, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMTRA57VPH5WXPQCN7BKTDRQLUYJANCNFSM4MXCZOSA .

carlonluca commented 4 years ago

I don't think anyone should ever base his own business on master in any case. That is the reason why tags exist. If you have a version you trust, I'd tag it. In any case please don't trust this pull request too much. I only used it in my specific use case. I would not integrate before someone else confirms this is ok.

christiansalazar commented 4 years ago

i agree (luca). it is complex, it takes more time than i have available in order to be responsible with the people who trust on this solution. That's why i didnt touch anything (including PR).

El jue., 7 may. 2020 a las 18:47, Luca Carlon (notifications@github.com) escribió:

I don't think anyone should ever base his own business on master in any case. That is the reason why tags exist. If you have a version you trust, I'd tag it. In any case please don't trust this pull request too much. I only used it in my specific use case. I would not integrate before someone else confirms this is ok.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/freesoftwarefactory/parse-multipart/pull/27#issuecomment-625534879, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMTRA7KP2D7OQDHIWUVYJLRQM27LANCNFSM4MXCZOSA .

eugenelim commented 3 years ago

@christiansalazar If @Satwa were to just clone the repo on his own time, would you mind updating your Readme.md to link to @Satwa new repo?

@Satwa would you be ok to be primary maintainer for the new repo if @christiansalazar is ok?

vbackeberg commented 3 years ago

I can confirm this issue. I was about to use your modification which looks reasonable but I decided to use a workaround:

Always provide the missing filename by sending your payload as a blob.

MDN even has an example for a JSON: https://developer.mozilla.org/en-US/docs/Web/API/Blob#creating_a_blob

Maybe this is helpful to other people running into this issue but not wanting to change the library.

Anyway, big thanks to @christiansalazar for providing this library in the first place and that discussion here also helped me. 👍

christiansalazar commented 3 years ago

thank you :) im sorry about not performing changes on this repo, i have no time to deal with necesary details to ensure stability after chaning its code. so, please read the open issues and commentaries to find the right solution for your case. :)