-
Notifications
You must be signed in to change notification settings - Fork 82
Handle missing filename header #27
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
base: master
Are you sure you want to change the base?
Conversation
|
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:
#27
Commit Summary
- Handle missing filename header.
File Changes
- *M* multipart.js
<https://github.com/freesoftwarefactory/parse-multipart/pull/27/files#diff-457a4c3b154e8caf1eb6b1b953aad9f9>
(2)
Patch Links:
- https://github.com/freesoftwarefactory/parse-multipart/pull/27.patch
- https://github.com/freesoftwarefactory/parse-multipart/pull/27.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#27>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMTRA5DAVHCNZKSKV7OU23RPLC2FANCNFSM4MXCZOSA>
.
|
|
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 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
<#27 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMTRA2OG5LX3BKUKHFLKBDRQJ5QVANCNFSM4MXCZOSA>
.
|
|
Of course, I can give few hours per month to look at issues and PRs on this package |
|
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
<#27 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMTRA5L57UDKN4GZUQE5YTRQLQ3DANCNFSM4MXCZOSA>
.
|
|
but if the test data test passes, this shouldn't break anything? |
|
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
<#27 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMTRAZHI6RMHP3FFHSIYUDRQLSHBANCNFSM4MXCZOSA>
.
|
|
that's the game of open-source, they'll either make a PR or open an issue |
|
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
<#27 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMTRA57VPH5WXPQCN7BKTDRQLUYJANCNFSM4MXCZOSA>
.
|
|
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. |
|
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
<#27 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMTRA7KP2D7OQDHIWUVYJLRQM27LANCNFSM4MXCZOSA>
.
|
|
@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? |
|
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: 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. 👍 |
|
thank you :) |
Quick fix to avoid crashing when filename is not specified.