Skip to content
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

[api] Add event-based ability to modify pre-flight proxy requests. #673

Merged
merged 1 commit into from Jul 20, 2014

Conversation

msporny
Copy link
Contributor

@msporny msporny commented Jul 18, 2014

Problem Description

In some proxy scenarios, it's helpful to modify the outgoing proxy request before the headers are sent. For example, if a developer would like to add a few custom proxy-specific headers, the cleanest way to do this is to add the headers to the http.ClientRequest proxyReq object. Unfortunately, there is no way to access the proxyReq object pre-flight.

The problem manifested itself when I attempted to digitally sign an outgoing proxy request. In order to perform a digital signature on the HTTP headers using the HTTP Signatures specification you have to have access to all of the finalized headers. Since http-proxy modifies/adds headers, the digital signature should only be performed after all such modifications have been made but before the data is piped.

Solution

Add a proxyReq event that a proxy server can listen to in order to modify the request before it is sent. The event handler function has the following signature: function(http.ClientRequest proxyReq, http.IncomingMessage req, http.ServerResponse res, Object options). This enables the developer to modify the outgoing proxy connection in a variety of very flexible ways.

Potential Issues

  1. If options.forward is specified, modification isn't supported. I think that's how it should work, but core implementers may disagree with that approach.
  2. There is currently no way to take the body of the HTTP message into account when the event is called, which means that performing a header modification on the body isn't possible. For example, it's not possible to add a header specifying a Digest SHA-256 sum for the body. Adding this feature could create scalability nightmares as one would need to do a full read of the data stream in some cases (which could be multiple megabytes or gigabytes of data).

@jcrugzz
Copy link
Contributor

jcrugzz commented Jul 20, 2014

@msporny thanks for making the changes! LGTM. Regarding the potential issue #2, this would need to be a transform stream of sorts that automatically modified headers and passed it along to the proxy. I haven't figured out how this would be done generically yet but its something I have in mind.

jcrugzz added a commit that referenced this pull request Jul 20, 2014
[api] Add event-based ability to modify pre-flight proxy requests.
@jcrugzz jcrugzz merged commit 3e8ee04 into http-party:master Jul 20, 2014
@msporny
Copy link
Contributor Author

msporny commented Jul 20, 2014

Thanks for the merge @jcrugzz :).

@dlongley any thoughts on addressing issue #2 generically since we're probably going to need to do this at some point in the near future?

@dlongley
Copy link

@msporny, well, from a quick glance at the code it seems like there are several options now just after this approach, for addressing number 2:

  1. For producing something like a digest on message bodies that are too long to buffer into memory, you could attach an event listener to proxyReq and then attach a data event listener to the incoming request that will digest the data and, when finished, call proxyReq.addTrailers with the appropriate digest header.
  2. For message bodies that can be buffered, you could do the same -- except set options.buffer in the proxyReq handler to some stream that also gets populated by listening to events from req. There may be a nicer API that could be exposed for this, but it seems like it would work as is if you needed this behavior. But again, that's just from a quick glance, I didn't write any code to test this.

Edit: Actually, it looks like some tweaking would need to happen to allow options.buffer to be set in the event handler. Maybe something similar to what @jcrugzz suggested could be added to allow a transform stream to be inserted between options.buffer or req and the proxyReq -- or this stream could always be present and, by default, it performs no transformation.

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

Successfully merging this pull request may close these issues.

None yet

4 participants