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

Memory leak in Abourget fork #241

Closed
SmashGuy2 opened this issue Sep 11, 2017 · 1 comment
Closed

Memory leak in Abourget fork #241

SmashGuy2 opened this issue Sep 11, 2017 · 1 comment

Comments

@SmashGuy2
Copy link

Hi everyone,

Not sure if this is the right place to post this but github does not allow me to submit issues to a fork.

There appears to be a memory leak in the abourget version, specifically relating to the following code at ctx.go (https://github.com/abourget/goproxy/blob/master/ctx.go):

  clientTlsReader := bufio.NewReader(rawClientTls)
  for !isEof(clientTlsReader) {
  	// This reads a normal "GET / HTTP/1.1" request from the tunnel, as it thinks its
  	// talking directly to the server now, not to a proxy.
  	subReq, err := http.ReadRequest(clientTlsReader)

What happens here is that various sites do not properly close the TLS handshake and so the for loop continues on indefinitely. I've placed a timeout of 10 seconds on the loop, which helps a great deal. However, some connections cause the ReadRequest() to hang indefinitely. There is actually a comment above this section of code which describes the same problem.

I'm not sure why this section of code is in a for loop. Seems that a simple if...then statement would be a better way of handling this. And if anyone has any ideas of how to properly fix this, please let me know. Otherwise, I'll probably just figure out a way to timeout the connection and kill it.

@SmashGuy2
Copy link
Author

Looks like this has already been fixed in the elazarl version:

#209

Setting the header in ctx.go:forwardMITMResponse() seems to do the trick!

Guess this begs the question... are there still any plans for you guys to merge your versions?

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

1 participant