koding / tunnel

Tunnel proxy package in Go
BSD 3-Clause "New" or "Revised" License
322 stars 71 forks source link

Close proxy connections when finished with transfer #42

Closed AkeemMcLennon closed 6 years ago

AkeemMcLennon commented 7 years ago

This PR closes the proxy connections after a transfer is completed.

This specifically addresses situations where the browser will wait for the connection to close explicitly before fully loading the page (e.g. when Content-Length=-1).

I've left the source connection open since this is used to maintain keep alive connections on the client side from my understanding of the net.http/response Response docs.

mmatczuk commented 7 years ago

@rjeczalik @AkeemMcLennon you could do like this https://github.com/mmatczuk/go-http-tunnel/blob/master/utils.go#L18, it works provided that you call transfer(dst, src) and transfer(src, dst).

AkeemMcLennon commented 7 years ago

Hello everyone, thanks for the feedback! I've done a little bit more research into why this bug is happening and it looks like it can only be reproduced when the proxied server is using HTTP 1.0 and the Content Length is unspecified (or -1).

It appears that most http 1.0 clients will assume the connection is closed after it has reached the end of the body. However, in http 1.1, this is no longer the case, and a "Connection: close" header must be sent explicitly. Presumably, the http Response takes care of this when you call NetConn.Close(). Normally, when the tunnel proxies another http 1.1 server, this header just gets copied anyway.

AkeemMcLennon commented 7 years ago

@mmatczuk Thanks, I'll give that a try when I get back! However, I've noticed that the docs say I should just use Close(). I'm new to Golang so I'm not sure what the exact difference is. Could you explain why you chose one over the other? See https://golang.org/pkg/net/#TCPConn.CloseRead

rjeczalik commented 7 years ago

Could you explain why you chose one over the other?

See my response here.

AkeemMcLennon commented 7 years ago

Excuse the broken promises. I haven't forgotten, I've just been a bit busy lately :-/