Skip to content
This repository has been archived by the owner on Apr 20, 2019. It is now read-only.

pez #4

Closed
jonathanong opened this issue Sep 16, 2014 · 9 comments
Closed

pez #4

jonathanong opened this issue Sep 16, 2014 · 9 comments

Comments

@jonathanong
Copy link

looks like you're going to be basing it off of multiparty. why not just continue maintaining multiparty instead (like with qs)?

@hueniverse
Copy link
Contributor

Is multiparty looking for a new home? Happy to follow with the same pattern as with qs (eg. moving to hapijs and taking over). I consider this a core module and want to better align it with hapi needs, hence the fork.

@jonathanong
Copy link
Author

i don't know. @dougwilson seems to be maintaining it right now as well. @andrewrk?

@hueniverse
Copy link
Contributor

That's what I thought. I am going to end up rewriting the whole thing. I find multiparty to manage state all over the place which is very hard to follow as a new maintainer.

@andrewrk
Copy link

multiparty has good test coverage and is actively maintained thanks to @dougwilson. If there's something it does wrong or a feature you need that it does not provide, consider contributing a patch instead of forking.

If the problem is that it is not tightly integrated into hapi, then consider redesigning your architecture to adhere to the principle of loose coupling

@hueniverse
Copy link
Contributor

We've had good experience with it. But I'm moving hapi into a more isolated state with minimum external (non hapijs) dependencies. Security and control are a major issue for many of our big customers and while this is not really in the spirit of an open modules system, it is the best solution I have.

Honestly, I'm having a hard time following the multiparty codebase. This is my problem. When there are patches or bug fixes, I can't really evaluate them, figure out security implications, etc. This means I'm basically importing unknown code into the framework in an area that's both hot and critical.

Ultimately, I don't expect pez to be an actual fork. It will be a brand new codebase. I am probably going to remove the code in there now and start from scratch (after spending 2 days trying to work with it).

@andrewrk
Copy link

multiparty started as a fork of formidable. If you think multiparty code is hard to follow, I challenge you to crack open formidable.

I agree that there could be something to be gained from a complete rewrite. There's also busboy to check out, or dicer.

My advice to you, if you go the route of rewriting it:

  • completely toss out all of multiparty's code and start fresh. don't start with a copy paste
  • read the multipart rfc specs yourself
  • make sure you pass all of multiparty's test suite and maybe busboy's too
  • do not give into the pressure of people wanting built in support for turning e.g. ?foo[0]=a&foo[1]=b into ["a", "b"]. This is dangerous and error prone, and I have no idea why this "feature" is so desirable.

I'm sure multiparty's code base could be a lot cleaner, but when it comes down to it, it's a finite state machine parser, and I'm having a hard time imagining that you're going to do anything different.

@dougwilson
Copy link

So yea, I don't think multiparty is looking for a new home, as already stated :) Really, if the hapijs team wants to make a whole new multipart parser, then let them be :) I certainly understand that they may have different requirements that may necessitate creating a new parser. They may end up running into the same bugs solved in other modules, but that's just the cost of recreating it :) There is nothing super amazing about multiparty (in fact, it parses way too liberally than I would like), but it works, haha.

It would be nice if the hapijs team were to discover some exploit while developing if they let the other multipart owners know, but I don't expect it, especially since they would not even be using the others :)

TL;DR if the hapijs team wants to make a new parser, let them in my opinion :)

@dougwilson
Copy link

Also, I checked the pez repo and the code is under a BSD-3 clause license, which is nice and permissive like MIT, so they're not even "hoarding" their code ;)

@hueniverse
Copy link
Contributor

Pez is done and now part of hapi. It is about 5% slower than multiparty right now before any optimizations.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants