Migrate the ecosystem

[kibertoad]

Prerequisites

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

Issue

Now that 1.0.0 is out, we should go through https://www.npmjs.com/package/busboy and submit migration PRs to projects that have non-insignificant amount of users.

Done: fastify-multipart, multer.

[Uzlopak]

@dougwilson is watching this fork. Do you plan to migrate the projects under your maintenance to @fastify/busboy?

[kibertoad]

@dougwilson We would be happy to create PRs for that, if you would give us a list of projects you would like migrated. Note that it would probably be a semver major, since I assume most of your project have < Node 10 support.

[Uzlopak]

We should also think about the dependents of dicer. Like firebase-admin of google. Can they actually import Dicer from @fastify/busboy?

[kibertoad]

@Uzlopak That would be a wrong approach either way. If that's something that we want to do, we'd need to extract dicer to fastify-dicer.

[Uzlopak]

Maybe we should first inform them about the dicer issues and if there is a demand, we extract it to fastify-dicer so that they can consume it.

[kibertoad]

Sounds good!

[jaydenseric]

Having worked with busboy for years now in graphql-upload, it's exciting to see a motivated team taking on the woefully neglected multipart request stream parsing problem space. I've been unhappy with busboy for some time due to the low quality readme/documentation, lack of maintenance (ancient codebase, stagnant PRs), and deliberate rejection of semver:

As far as semver goes, none of my projects follow semver. It's easy enough to do ~0.3.0 to stay on that branch without any intentional breaking changes.
— mscdex/busboy#188 (comment)

The dicer dependency is pinned, which is an anti-pattern:

https://github.com/mscdex/busboy/blob/5ef3f9b1d08e7ccb7c9b3690e4c071089e66285c/package.json#L7

It doesn't sit well that massive dependencies are copy-pasted in, instead of being managed as regular auditable npm package dependencies:

• https://unpkg.com/browse/busboy@0.3.1/deps/encoding/encoding.js
• https://unpkg.com/browse/busboy@0.3.1/deps/encoding/encoding-indexes.js

Also, the 739 kB install size is over the top. Tests and other superfluous files are being published:

https://unpkg.com/browse/busboy@0.3.1/

There is little hope that busboy will keep up with modern paradigms like pure ESM packages, or evolve its API for standard web streams, async iterators, etc.

There just hasn't really been a better choice, which is surprising considering how essential a library like this is to building for web.

That said, it would be great to hear a bit more about the motivation for this fork, vs:

• Taking over or contributing to maintenance of the busboy package. Was this considered, proposed, or rejected? This would bring the greatest benefit to the existing ecosystem of packages dependent on busboy.
• Publishing a fresh package. The Fastify project could probably pull together the expertise and resources to build a production grade solution from scratch, using modern syntax and (hopefully) best practices. Personally I would rewrite busboy so much that it couldn't really be called a fork anymore.

[kibertoad]

Taking over or contributing to maintenance of the busboy package. Was this considered, proposed, or rejected? This would bring the greatest benefit to the existing ecosystem of packages dependent on busboy.

It wasn't proposed, and in a hindsight it probably was a mistake. My assumption was that since repo is mostly hibernating these days, mscdex must be no longer actively engaged with it, which is actually not true - he still comments on it very actively. We did ask him if he would be open to transfer npm package name to fastify org to avoid fragmentation, but there was no reply.

That said - @mscdex - if you agree with the general direction of the package that you can already see in this fork, would you consider accepting changes that happened here so far and considering maintainers of @fastify/busboy for the position of co-maintainers of busboy, so that we could combine our forces?

The Fastify project could probably pull together the expertise and resources to build a production grade solution from scratch

Unfortunately, you seem to be overestimating the amount of resources that fastify project can realistically dedicate to such an effort. fastify-busboy was mostly put together by @Uzlopak and yours truly, and incorporated plenty of pending contributions from busboy community, neither of us being funded by fastify or anyone else, simply because it felt to be the right thing to do (it traces back to this discussion, if you are curious). Fellow fastify members reviewed the code and offered valuable suggestions, and some extra optimizations, but that's about it. There is simply no capacity for anything more significant at this point.

Any ideas or PRs that would help improve busboy, including significant rewrites of it, are very welcome, btw.

[Uzlopak]

@mscdex
We would be happy if we could take care of busboy, dicer and searchstream under the flag of fastify.

@jaydenseric

I kind of agree with you. But you have to consider, that mscdex did not update dicer even though it had two bugs which could result in DoS-Attacks as they could hang or crash the service, despite having two PRs in Dicer for months, which are solving those attack vectors. So to consider the packages being not actively maintained by mscdex is not unreasonable. I dont blame him, as he has hundreds of successful projects and keeping them all up to date is time consuming.

But for my use case, I wrote an oauth2-authorization-server with fastify, it would mean a security issue in a critical part of my infrastructure. I dont want, that my deployment has an avoidable attack vector.

On the other hand, mscdex encourages to fork his projects. So I conclude a fragmentation is considered OK by mscdex.

But I can tell you that we put alot of effort to remove security issues, increase the code coverage and to extend the features of our busboy fork.
We are now at 95 % code coverage. We are actually now in the position to refactor busboy. We can also ensure semantic versioning, to increase the developer experience.

I would have preferred that we take maintenance of busboy, dicer and searchstream project, but it did not happen.

But hey... you can never make everybody happy. We offer here an alternative with the potential that it gets step by step refactored.

[Uzlopak]

@jaydenseric
addendum:
I forgot to mention, that I also proposed to reimplement busboy from the scratch as the interfaces of busboy definetely are a success story. But we have simply currently not the capacity to rewrite it from scratch to make the project state of the art. But I still think that we did a good job with our rare resources setting the foundation for future changes.

So I also encourage you to provide us with PRs or to tell what we could do better by opening issues.

[jaydenseric]

node-fetch now implements a multipart parser written from scratch in only a couple hundred lines:

https://github.com/node-fetch/node-fetch/blob/109bd21313c277f043089f8c38b1a716c39ff86f/src/utils/multipart-parser.js#L36-L318

I haven't looked close enough at it to tell yet if that effort is something that could replace busboy in graphql-upload, but surely it's an indication of what's possible regarding line count.

Maybe their code could be abstracted out into a generic multipart parser package?

[Uzlopak]

I know it seems quite intriguing to adapt that as the node-fetch implementation is small and simple and I like that.

But our current structure actually provides a good separation of concerns. The node-fetch implementation is an amalgam of what dicer and searchstream does. It would imho make it harder to improve the code if we would mangling all to one Stream. I think. But maybe I am wrong (which is OK :D)

[harshagarwal00]

@Uzlopak @kibertoad : please do check firebase/firebase-admin-node#1512
There seems to be some interest.. to have fastify/dicer

[LinusU]

I've updated Multer 2.x to use this fork now, whilst 1.x have been deprecated, and 1.4.4-lts.1 have been updated to use Busboy 1.x 😅

Now that the original Busboy is being actively maintained again, are you committed to maintaining this fork long term? I would love some input on what version Multer 2.x should use before we make a stable release...

[Uzlopak]

We are willing to maintain this package long term