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

Can't chain outputs from one handler to another #352

Open
dmjones opened this issue Jul 15, 2019 · 4 comments
Open

Can't chain outputs from one handler to another #352

dmjones opened this issue Jul 15, 2019 · 4 comments

Comments

@dmjones
Copy link
Contributor

dmjones commented Jul 15, 2019

I was hoping to construct a chain of filters by doing something like this:

proxy := goproxy.NewProxyHttpServer()
proxy.OnRequest().Do(filter1)
proxy.OnRequest().Do(filter2)

However, I've noticed filters get passed the original HTTP request, rather than the result from an earlier filter in the chain:

// proxy.go:58
func (proxy *ProxyHttpServer) filterRequest(r *http.Request, ctx *ProxyCtx) (req *http.Request, resp *http.Response) {
	req = r
	for _, h := range proxy.reqHandlers {
		req, resp = h.Handle(r, ctx)   // <-------- r, not req

		if resp != nil {
			break
		}
	}
	return
}

Notice how h.Handle is invoked on r not req. Is this intentional or an error?

In the meantime, I can work around this by creating one handler that performs the chaining I need.

@elazarl
Copy link
Owner

elazarl commented Sep 11, 2019

IIRC I was thinking that one should concatenate the handler creation, to remove ambiguity.

Do you have a better idea? Would the reader understand the code snippet you wrote better? I do not have particular opinion, you might be right here.

@jgroffen
Copy link

jgroffen commented May 1, 2020

Hello @elazarl,

I hit the same problem and agree with @dmjones, the reason handlers receive AND return the request object is to give handlers an opportunity to change the request, and have those changes persist. Keep in mind that a common pattern here is to create and return a new request, replacing the one passed in to the handler.

If you pass the original request object (r in this case) for each call to a handler then any changes the handlers make are lost.

This is of course except for the last time a handler is called - as only the last return value from h.Handler is retained and returned by filterRequest,

This led to some buggy behavior where once I chained more than one handler - I would lose changes to the request made by all but the last handler called. This wasn't easy to diagnose as it appears to work properly when you only apply one handler.

In my example I have two different activities, both of which need to store information in context on the request, then read that information in the response. I did this with a call that looks something like this:

goproxy.OnRequest(reqConditionHandler1).DoFunc(requestHandler1)
goproxy.OnRequest(reqConditionHandler1).DoFunc(requestHandler2)

func requestHandler1(req *http.Request, ctx *goproxy.ProxyCtx) (req *http.Request, resp *http.Response) {
    # create some context information
    ...
    req = req.WithContext(context.WithValue(req.Context(), ContextKey1, someContextValue))
}

func requestHandler2(req *http.Request, ctx *goproxy.ProxyCtx) (req *http.Request, resp *http.Response) {
    # create some other context information
    ...
    req = req.WithContext(context.WithValue(req.Context(), ContextKey2, someOtherContextValue))
}

... but only one of the contexts survived on the request, based on whichever handler happened to be called last.

I checked the filterResponse method in proxy.go to see if it has the same problem - I spent some more time looking at this method and it appears to behave correctly, copying respOrig to resp (the return value) and then using resp in calls to handlers AND setting resp to the handler result.

EDIT: The example was updated to include reqConditionHandler1 and reqConditionHandler2, as the issue is related to filterRequest, which is only called if you are applying filters!

@elazarl
Copy link
Owner

elazarl commented May 1, 2020

Wouldn't

goproxy.OnRequest().DoFunc(requestHandler1).DoFunc(requestHandler2)

work for your case?

What would a person that do not want to concatentate requests do?

@jgroffen
Copy link

jgroffen commented May 8, 2020

Hello @elazarl, sorry about the delay in responding,

Also I must apologise as my example has a mistake in it. The issue is with filterRequest in proxy.go, so my example should show that I'm applying filters. Assume in the example below that reqConditionHandler1 and reqConditionHandler2 are valid goproxy.ReqConditionFunc variables:

goproxy.OnRequest(reqConditionHandler1).DoFunc(requestHandler1)
goproxy.OnRequest(reqConditionHandler2).DoFunc(requestHandler2)

func requestHandler1(req *http.Request, ctx *goproxy.ProxyCtx) (req *http.Request, resp *http.Response) {
    # create some context information
    ...
    req = req.WithContext(context.WithValue(req.Context(), ContextKey1, someContextValue))
}

func requestHandler2(req *http.Request, ctx *goproxy.ProxyCtx) (req *http.Request, resp *http.Response) {
    # create some other context information
    ...
    req = req.WithContext(context.WithValue(req.Context(), ContextKey2, someOtherContextValue))
}

Sorry about that - I over-simplified my example. Another way it is over-simplified is it's a bit misleading as to my situation. I have developed a system where I can apply 'mods' to a proxy to change the proxy behavior. Each mod may independently contribute separate OnRequest handlers to be called during request processing ... optionally with filters to limit what requests will be handled by what handlers. I can't explicitly chain them because which mods get included and what filters are applied are configuration-managed, and contributed to the handler chain from separate interface implementations of my 'Mod' interface.

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

No branches or pull requests

3 participants