From 6f196510b81e5781703c5e1781b1c21637d46eb6 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Mon, 22 Feb 2021 15:02:15 +0100 Subject: [PATCH 01/11] Add log on transfer close --- pkg/http-tunnel/server/tcpproxy.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/pkg/http-tunnel/server/tcpproxy.go b/pkg/http-tunnel/server/tcpproxy.go index 456d0398..a35b0727 100644 --- a/pkg/http-tunnel/server/tcpproxy.go +++ b/pkg/http-tunnel/server/tcpproxy.go @@ -6,11 +6,12 @@ package server import ( "fmt" - "github.com/NodeFactoryIo/vedran/pkg/http-tunnel" - "github.com/NodeFactoryIo/vedran/pkg/http-tunnel/proto" - log "github.com/sirupsen/logrus" "io" "net" + + tunnel "github.com/NodeFactoryIo/vedran/pkg/http-tunnel" + "github.com/NodeFactoryIo/vedran/pkg/http-tunnel/proto" + log "github.com/sirupsen/logrus" ) // TCPProxy forwards TCP streams. @@ -80,7 +81,6 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage }).Error("dial failed", err) return } - defer local.Close() if err := tunnel.KeepAlive(local); err != nil { clogger.WithFields(log.Fields{ @@ -95,6 +95,7 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage "src": target, }) transfer(flushWriter{w}, local, loggerWithContext) + clogger.Info("Transfer close called") close(done) }() @@ -105,6 +106,12 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage transfer(local, r, loggerWithContext) <-done + err = local.Close() + if err != nil { + clogger.Errorf("Local close failed because of %v") + } + + clogger.Info("Transfer close called and magic happeneded") } func (p *TCPProxy) localAddrFor(hostPort string) string { From 43c8b332a605391c84d7f6ea5716ac59f0c58590 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Mon, 22 Feb 2021 15:35:37 +0100 Subject: [PATCH 02/11] Undo defer --- pkg/http-tunnel/server/tcpproxy.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/http-tunnel/server/tcpproxy.go b/pkg/http-tunnel/server/tcpproxy.go index a35b0727..e562eeed 100644 --- a/pkg/http-tunnel/server/tcpproxy.go +++ b/pkg/http-tunnel/server/tcpproxy.go @@ -81,6 +81,7 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage }).Error("dial failed", err) return } + defer local.Close() if err := tunnel.KeepAlive(local); err != nil { clogger.WithFields(log.Fields{ @@ -106,10 +107,12 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage transfer(local, r, loggerWithContext) <-done - err = local.Close() - if err != nil { - clogger.Errorf("Local close failed because of %v") - } + /* + err = local.Close() + if err != nil { + clogger.Errorf("Local close failed because of %v") + } + */ clogger.Info("Transfer close called and magic happeneded") } From 99038ecd4494bd371438f44fefac550b323bc1df Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Mon, 22 Feb 2021 16:43:58 +0100 Subject: [PATCH 03/11] Add log before and after tunnel copy --- pkg/http-tunnel/server/utils.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/http-tunnel/server/utils.go b/pkg/http-tunnel/server/utils.go index b936074d..ec695844 100644 --- a/pkg/http-tunnel/server/utils.go +++ b/pkg/http-tunnel/server/utils.go @@ -5,15 +5,18 @@ package server import ( - log "github.com/sirupsen/logrus" "io" "net" "net/http" "strings" + + log "github.com/sirupsen/logrus" ) func transfer(dst io.Writer, src io.Reader, logger *log.Entry) { + log.Info("BEFORE COPY") n, err := io.Copy(dst, src) + log.Info("AFTER COPY") if err != nil { if (!strings.Contains(err.Error(), "context canceled") && !strings.Contains(err.Error(), "CANCEL")) && From 5f6353f1a1f66b8e359edf71af0e0dc82f5a8674 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Mon, 22 Feb 2021 17:00:27 +0100 Subject: [PATCH 04/11] Add explicit return to transfer --- pkg/http-tunnel/server/utils.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/pkg/http-tunnel/server/utils.go b/pkg/http-tunnel/server/utils.go index ec695844..4dbb5de4 100644 --- a/pkg/http-tunnel/server/utils.go +++ b/pkg/http-tunnel/server/utils.go @@ -15,16 +15,10 @@ import ( func transfer(dst io.Writer, src io.Reader, logger *log.Entry) { log.Info("BEFORE COPY") - n, err := io.Copy(dst, src) + n, _ := io.Copy(dst, src) log.Info("AFTER COPY") - if err != nil { - if (!strings.Contains(err.Error(), "context canceled") && - !strings.Contains(err.Error(), "CANCEL")) && - !strings.Contains(err.Error(), "stream closed") { - logger.Error("copy error ", err) - } - } log.Debugf("transferred %d bytes", n) + return } func setXForwardedFor(h http.Header, remoteAddr string) { From 2ff5f79ade6491d167356495c9359372394d294d Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Mon, 22 Feb 2021 17:10:30 +0100 Subject: [PATCH 05/11] Add close after after any transfer ends + --- pkg/http-tunnel/server/tcpproxy.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/http-tunnel/server/tcpproxy.go b/pkg/http-tunnel/server/tcpproxy.go index e562eeed..c612a606 100644 --- a/pkg/http-tunnel/server/tcpproxy.go +++ b/pkg/http-tunnel/server/tcpproxy.go @@ -105,6 +105,7 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage "src": target, }) transfer(local, r, loggerWithContext) + close(done) <-done /* From a52a1c0592999b1361527350c9cc8833785d2e42 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Tue, 23 Feb 2021 10:33:23 +0100 Subject: [PATCH 06/11] Remove transfer channel --- pkg/http-tunnel/server/tcpproxy.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/pkg/http-tunnel/server/tcpproxy.go b/pkg/http-tunnel/server/tcpproxy.go index c612a606..f21dd37f 100644 --- a/pkg/http-tunnel/server/tcpproxy.go +++ b/pkg/http-tunnel/server/tcpproxy.go @@ -81,7 +81,6 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage }).Error("dial failed", err) return } - defer local.Close() if err := tunnel.KeepAlive(local); err != nil { clogger.WithFields(log.Fields{ @@ -89,7 +88,6 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage }).Error("TCP keepalive for tunneled connection failed", err) } - done := make(chan struct{}) go func() { loggerWithContext := log.WithContext(p.logger.Context).WithFields(log.Fields{ "dst": msg.ForwardedHost, @@ -97,7 +95,6 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage }) transfer(flushWriter{w}, local, loggerWithContext) clogger.Info("Transfer close called") - close(done) }() loggerWithContext := log.WithContext(p.logger.Context).WithFields(log.Fields{ @@ -105,15 +102,10 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage "src": target, }) transfer(local, r, loggerWithContext) - close(done) - - <-done - /* - err = local.Close() - if err != nil { - clogger.Errorf("Local close failed because of %v") - } - */ + err = local.Close() + if err != nil + clogger.Errorf("Local close failed because of %v") + } clogger.Info("Transfer close called and magic happeneded") } From 94bef1e2040855692b553ea478d68e0e5d1c8482 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Tue, 23 Feb 2021 10:34:35 +0100 Subject: [PATCH 07/11] Lint --- pkg/http-tunnel/server/tcpproxy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/http-tunnel/server/tcpproxy.go b/pkg/http-tunnel/server/tcpproxy.go index f21dd37f..bae4bd71 100644 --- a/pkg/http-tunnel/server/tcpproxy.go +++ b/pkg/http-tunnel/server/tcpproxy.go @@ -103,7 +103,7 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage }) transfer(local, r, loggerWithContext) err = local.Close() - if err != nil + if err != nil { clogger.Errorf("Local close failed because of %v") } From 6a64f2463d7c8ff6f4afd50c03b802dd5961d87a Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Tue, 23 Feb 2021 10:54:31 +0100 Subject: [PATCH 08/11] Remove extra logs --- pkg/http-tunnel/server/tcpproxy.go | 6 ++---- pkg/http-tunnel/server/utils.go | 2 -- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/pkg/http-tunnel/server/tcpproxy.go b/pkg/http-tunnel/server/tcpproxy.go index bae4bd71..8fe3c736 100644 --- a/pkg/http-tunnel/server/tcpproxy.go +++ b/pkg/http-tunnel/server/tcpproxy.go @@ -94,7 +94,6 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage "src": target, }) transfer(flushWriter{w}, local, loggerWithContext) - clogger.Info("Transfer close called") }() loggerWithContext := log.WithContext(p.logger.Context).WithFields(log.Fields{ @@ -102,12 +101,11 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage "src": target, }) transfer(local, r, loggerWithContext) + err = local.Close() if err != nil { - clogger.Errorf("Local close failed because of %v") + clogger.Errorf("Transfer close failed because of %v") } - - clogger.Info("Transfer close called and magic happeneded") } func (p *TCPProxy) localAddrFor(hostPort string) string { diff --git a/pkg/http-tunnel/server/utils.go b/pkg/http-tunnel/server/utils.go index 4dbb5de4..73a03037 100644 --- a/pkg/http-tunnel/server/utils.go +++ b/pkg/http-tunnel/server/utils.go @@ -14,9 +14,7 @@ import ( ) func transfer(dst io.Writer, src io.Reader, logger *log.Entry) { - log.Info("BEFORE COPY") n, _ := io.Copy(dst, src) - log.Info("AFTER COPY") log.Debugf("transferred %d bytes", n) return } From bcbd9357fc8559a2aaeb73c36fc3811b6ed3f39c Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Tue, 23 Feb 2021 10:54:45 +0100 Subject: [PATCH 09/11] Remove extra return --- pkg/http-tunnel/server/utils.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/http-tunnel/server/utils.go b/pkg/http-tunnel/server/utils.go index 73a03037..0bd4186a 100644 --- a/pkg/http-tunnel/server/utils.go +++ b/pkg/http-tunnel/server/utils.go @@ -16,7 +16,6 @@ import ( func transfer(dst io.Writer, src io.Reader, logger *log.Entry) { n, _ := io.Copy(dst, src) log.Debugf("transferred %d bytes", n) - return } func setXForwardedFor(h http.Header, remoteAddr string) { From 740cbb2e3f98d0c97736526503309dc02de7dc39 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Tue, 23 Feb 2021 10:57:51 +0100 Subject: [PATCH 10/11] Update CHANGELOG --- CHANGELOG.md | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20395e9c..ce4b781f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,17 @@ # Changelog -## [v0.5.3]((https://github.com/NodeFactoryIo/vedran/tree/HEAD)) -[Full Changelog](https://github.com/NodeFactoryIo/vedran/compare/v0.5.2...HEAD) +## [unreleased]((https://github.com/NodeFactoryIo/vedran/tree/HEAD)) +[Full Changelog](https://github.com/NodeFactoryIo/vedran/compare/v0.5.3...HEAD) + +### Added + +### Fix +- Fix tunnel tcp connections not closing after requests finish [\#197](https://github.com/NodeFactoryIo/vedran/pull/197) ([mpetrun5](https://github.com/mpetrun5)) + +### Changed + +## [v0.5.3]((https://github.com/NodeFactoryIo/vedran/tree/v0.5.3)) +[Full Changelog](https://github.com/NodeFactoryIo/vedran/compare/v0.5.2...v0.5.3) ### Added From 3c306ab4160e4630500858880ec80bf49708fc6b Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Tue, 23 Feb 2021 11:03:55 +0100 Subject: [PATCH 11/11] Lint --- pkg/http-tunnel/server/tcpproxy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/http-tunnel/server/tcpproxy.go b/pkg/http-tunnel/server/tcpproxy.go index 8fe3c736..53777372 100644 --- a/pkg/http-tunnel/server/tcpproxy.go +++ b/pkg/http-tunnel/server/tcpproxy.go @@ -104,7 +104,7 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage err = local.Close() if err != nil { - clogger.Errorf("Transfer close failed because of %v") + clogger.Errorf("Transfer close failed because of %v", err) } }