Net module based proxy #127

Closed
Marak opened this Issue Oct 10, 2011 · 13 comments

Comments

Projects
None yet
8 participants
Contributor

Marak commented Oct 10, 2011

It look's like @substack implemented a first pass at doing the tcp/net based http proxying ( which calls ry's http parser directly).

We should pull this in or use it as a reference for implementing the fast proxying scenarios we were discussing ( not needing the whole request, but just enough to route it ).

https://github.com/substack/bouncy/blob/master/index.js

+1 If you need some more hooks to make using bouncy as a library easier I can oblige.

Contributor

Marak commented Oct 10, 2011

I think it's probably just going to be put into the core proxy logic. We've been talking about this for a while, we just haven't been that motivated since the performance we already are getting is still really good.

Contributor

tj commented Oct 10, 2011

+1 for @substack's being a lib

0x00A commented Oct 10, 2011

+1 for substack lib, its a kernel, and at a glance looks easy to maintain. (this is just my immediate reaction, need to look at the code in more depth)

Contributor

indutny commented Oct 11, 2011

Do you have a benchmark for bouncy. While codebase is much cleaner than http-proxy's - it's still using the same parser and headers generator.

Contributor

tj commented Oct 11, 2011

I would remove the hasOwnProperty calls, unnecessary penalty IMO, if you augment Object.prototype with x-forwarded-for you deserve for things to break :D

0x00A commented Oct 11, 2011

@visionmedia agreed, there is a time and place for hasOwnProperty

The .hasOwnProperty() is just to see whether opts has those fields since if you set them to falsey values they aren't sent.

@indutny: https://gist.github.com/1275259 and https://github.com/substack/bouncy/tree/master/bench

Contributor

indutny commented Oct 11, 2011

Well , that's much more argumented :)

0x00A commented Oct 11, 2011

@substack .hasOwnProperty() might not be the best way to do that http://jsperf.com/hasownpropvsbool #maxPerf! >=)

Owner

indexzero commented Oct 11, 2011

We will not be taking bouncy as a dependency. The approach of using a TCP proxy has its merits, which I have discussed with @mikeal at length.

There is an experimental branch of node-http-proxy which exposes a transparent balancing proxy that uses this approach. I started work on this back in April I think: https://github.com/nodejitsu/node-http-proxy/blob/experimental/lib/balancing-proxy.js

Doing things on a TCP level breaks many of the existing HTTP-level APIs which users rely on in connect and/or express, so this won't be an all or nothing change.

I would welcome pull requests on the experimental branch which also allow for x-forwarded-* headers and sec-websocket-* header rewriting.

indexzero closed this Oct 11, 2011

Contributor

jfhbrook commented Oct 12, 2011

Doing things on a TCP level breaks many of the existing HTTP-level APIs which users rely on in connect and/or express

Sorry for being late to the party, but there isn't any way to get http req/res objects from a tcp connection? No way to "upgrade"?

mikeal commented Oct 12, 2011

is this ticket for a "TCP based proxy" or specifically for pulling in bouncy as a dep?

because I think all the issue you have with bouncy could still be dealt with if you just implemented it a little differently.

some of your features might not be doable, but some of your features are, to put it frankly, kind of retarded. i know some people want to buffer every response in to memory and mutate it but, well, that's dumb :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment