From b5f15f4aea27fa8769780f4a4944f7fdcbc23451 Mon Sep 17 00:00:00 2001 From: Mostafa Moradian Date: Fri, 9 Jun 2023 01:14:12 +0200 Subject: [PATCH 1/3] Check if connection is not nil before extracting the remote and local addresses --- cmd/run.go | 4 ++-- network/client.go | 18 ++++++++++++++++++ network/proxy.go | 16 ++++++++-------- network/utils.go | 34 ++++++++++++++++++++++++++++------ 4 files changed, 56 insertions(+), 16 deletions(-) diff --git a/cmd/run.go b/cmd/run.go index 6c756882..17902c2b 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -354,8 +354,8 @@ var runCmd = &cobra.Command{ attribute.String("sendDeadline", client.SendDeadline.String()), attribute.Bool("tcpKeepAlive", client.TCPKeepAlive), attribute.String("tcpKeepAlivePeriod", client.TCPKeepAlivePeriod.String()), - attribute.String("localAddress", client.Conn.LocalAddr().String()), - attribute.String("remoteAddress", client.Conn.RemoteAddr().String()), + attribute.String("localAddress", client.LocalAddr()), + attribute.String("remoteAddress", client.RemoteAddr()), ) if client.ID != "" { eventOptions = trace.WithAttributes( diff --git a/network/client.go b/network/client.go index 5e2ccadf..29c218a2 100644 --- a/network/client.go +++ b/network/client.go @@ -19,6 +19,8 @@ type IClient interface { Receive() (int, []byte, *gerr.GatewayDError) Close() IsConnected() bool + RemoteAddr() string + LocalAddr() string } type Client struct { @@ -245,3 +247,19 @@ func (c *Client) IsConnected() bool { return true } + +func (c *Client) RemoteAddr() string { + if c.Conn != nil && c.Conn.RemoteAddr() != nil { + return c.Conn.RemoteAddr().String() + } + + return "" +} + +func (c *Client) LocalAddr() string { + if c.Conn != nil && c.Conn.LocalAddr() != nil { + return c.Conn.LocalAddr().String() + } + + return "" +} diff --git a/network/proxy.go b/network/proxy.go index 437ad9e6..b769c06b 100644 --- a/network/proxy.go +++ b/network/proxy.go @@ -356,8 +356,8 @@ func (pr *Proxy) PassThrough(gconn gnet.Conn) *gerr.GatewayDError { pr.logger.Debug().Fields( map[string]interface{}{ "function": "proxy.passthrough", - "local": client.Conn.LocalAddr().String(), - "remote": client.Conn.RemoteAddr().String(), + "local": client.LocalAddr(), + "remote": client.RemoteAddr(), }).Msg("Client disconnected") client.Close() @@ -375,8 +375,8 @@ func (pr *Proxy) PassThrough(gconn gnet.Conn) *gerr.GatewayDError { pr.logger.Debug().Fields( map[string]interface{}{ "function": "proxy.passthrough", - "local": client.Conn.LocalAddr().String(), - "remote": client.Conn.RemoteAddr().String(), + "local": client.LocalAddr(), + "remote": client.RemoteAddr(), }).Msg("No data to send to client") span.AddEvent("No data to send to client") span.RecordError(err) @@ -581,8 +581,8 @@ func (pr *Proxy) sendTrafficToServer(client *Client, request []byte) (int, *gerr map[string]interface{}{ "function": "proxy.passthrough", "length": sent, - "local": client.Conn.LocalAddr().String(), - "remote": client.Conn.RemoteAddr().String(), + "local": client.LocalAddr(), + "remote": client.RemoteAddr(), }, ).Msg("Sent data to database") @@ -603,8 +603,8 @@ func (pr *Proxy) receiveTrafficFromServer(client *Client) (int, []byte, *gerr.Ga map[string]interface{}{ "function": "proxy.passthrough", "length": received, - "local": client.Conn.LocalAddr().String(), - "remote": client.Conn.RemoteAddr().String(), + "local": client.LocalAddr(), + "remote": client.RemoteAddr(), }, ).Msg("Received data from database") diff --git a/network/utils.go b/network/utils.go index b1ee7951..8797e109 100644 --- a/network/utils.go +++ b/network/utils.go @@ -58,18 +58,40 @@ func trafficData( fields []Field, err interface{}, ) map[string]interface{} { + if gconn == nil || client == nil { + return nil + } + data := map[string]interface{}{ - "client": map[string]interface{}{ - "local": gconn.LocalAddr().String(), - "remote": gconn.RemoteAddr().String(), - }, "server": map[string]interface{}{ - "local": client.Conn.LocalAddr().String(), - "remote": client.Conn.RemoteAddr().String(), + "local": client.LocalAddr(), + "remote": client.RemoteAddr(), }, "error": "", } + //nolint:nestif + if gconn != nil { + data["client"] = map[string]interface{}{} + if gconn.LocalAddr() != nil { + if client, ok := data["client"].(map[string]interface{}); ok { + client["local"] = gconn.LocalAddr().String() + } + } + if gconn.RemoteAddr() != nil { + if client, ok := data["client"].(map[string]interface{}); ok { + client["remote"] = gconn.RemoteAddr().String() + } + } + } + + if client != nil { + data["server"] = map[string]interface{}{ + "local": client.LocalAddr(), + "remote": client.RemoteAddr(), + } + } + for _, field := range fields { // field.Value will be converted to base64-encoded string. data[field.Name] = field.Value From e4242e1e5778b881a3515667724453fb6275268e Mon Sep 17 00:00:00 2001 From: Mostafa Moradian Date: Fri, 9 Jun 2023 01:29:37 +0200 Subject: [PATCH 2/3] Check if gnet connection is not nil before extracting the remote and local addresses --- network/client.go | 2 ++ network/proxy.go | 12 ++++++------ network/server.go | 24 ++++++++++++------------ network/utils.go | 42 ++++++++++++++++++++---------------------- 4 files changed, 40 insertions(+), 40 deletions(-) diff --git a/network/client.go b/network/client.go index 29c218a2..da24906f 100644 --- a/network/client.go +++ b/network/client.go @@ -248,6 +248,7 @@ func (c *Client) IsConnected() bool { return true } +// RemoteAddr returns the remote address of the client safely. func (c *Client) RemoteAddr() string { if c.Conn != nil && c.Conn.RemoteAddr() != nil { return c.Conn.RemoteAddr().String() @@ -256,6 +257,7 @@ func (c *Client) RemoteAddr() string { return "" } +// LocalAddr returns the local address of the client safely. func (c *Client) LocalAddr() string { if c.Conn != nil && c.Conn.LocalAddr() != nil { return c.Conn.LocalAddr().String() diff --git a/network/proxy.go b/network/proxy.go index b769c06b..ac7a042d 100644 --- a/network/proxy.go +++ b/network/proxy.go @@ -173,7 +173,7 @@ func (pr *Proxy) Connect(gconn gnet.Conn) *gerr.GatewayDError { fields := map[string]interface{}{ "function": "proxy.connect", "client": "unknown", - "server": gconn.RemoteAddr().String(), + "server": RemoteAddr(gconn), } if client.ID != "" { fields["client"] = client.ID[:7] @@ -533,7 +533,7 @@ func (pr *Proxy) BusyConnections() []string { connections := make([]string, 0) pr.busyConnections.ForEach(func(key, _ interface{}) bool { if gconn, ok := key.(gnet.Conn); ok { - connections = append(connections, gconn.RemoteAddr().String()) + connections = append(connections, RemoteAddr(gconn)) } return true }) @@ -554,8 +554,8 @@ func (pr *Proxy) receiveTrafficFromClient(gconn gnet.Conn) ([]byte, error) { pr.logger.Debug().Fields( map[string]interface{}{ "length": len(request), - "local": gconn.LocalAddr().String(), - "remote": gconn.RemoteAddr().String(), + "local": LocalAddr(gconn), + "remote": RemoteAddr(gconn), }, ).Msg("Received data from client") @@ -627,8 +627,8 @@ func (pr *Proxy) sendTrafficToClient( map[string]interface{}{ "function": "proxy.passthrough", "length": received, - "local": gconn.LocalAddr().String(), - "remote": gconn.RemoteAddr().String(), + "local": LocalAddr(gconn), + "remote": RemoteAddr(gconn), }, ).Msg("Sent data to client") span.RecordError(err) diff --git a/network/server.go b/network/server.go index af4fb683..310ccc7c 100644 --- a/network/server.go +++ b/network/server.go @@ -86,7 +86,7 @@ func (s *Server) OnOpen(gconn gnet.Conn) ([]byte, gnet.Action) { _, span := otel.Tracer("gatewayd").Start(s.ctx, "OnOpen") defer span.End() - s.logger.Debug().Str("from", gconn.RemoteAddr().String()).Msg( + s.logger.Debug().Str("from", RemoteAddr(gconn)).Msg( "GatewayD is opening a connection") pluginTimeoutCtx, cancel := context.WithTimeout(context.Background(), s.pluginTimeout) @@ -94,8 +94,8 @@ func (s *Server) OnOpen(gconn gnet.Conn) ([]byte, gnet.Action) { // Run the OnOpening hooks. onOpeningData := map[string]interface{}{ "client": map[string]interface{}{ - "local": gconn.LocalAddr().String(), - "remote": gconn.RemoteAddr().String(), + "local": LocalAddr(gconn), + "remote": RemoteAddr(gconn), }, } _, err := s.pluginRegistry.Run( @@ -141,8 +141,8 @@ func (s *Server) OnOpen(gconn gnet.Conn) ([]byte, gnet.Action) { // Run the OnOpened hooks. onOpenedData := map[string]interface{}{ "client": map[string]interface{}{ - "local": gconn.LocalAddr().String(), - "remote": gconn.RemoteAddr().String(), + "local": LocalAddr(gconn), + "remote": RemoteAddr(gconn), }, } _, err = s.pluginRegistry.Run( @@ -165,7 +165,7 @@ func (s *Server) OnClose(gconn gnet.Conn, err error) gnet.Action { _, span := otel.Tracer("gatewayd").Start(s.ctx, "OnClose") defer span.End() - s.logger.Debug().Str("from", gconn.RemoteAddr().String()).Msg( + s.logger.Debug().Str("from", RemoteAddr(gconn)).Msg( "GatewayD is closing a connection") pluginTimeoutCtx, cancel := context.WithTimeout(context.Background(), s.pluginTimeout) @@ -173,8 +173,8 @@ func (s *Server) OnClose(gconn gnet.Conn, err error) gnet.Action { // Run the OnClosing hooks. data := map[string]interface{}{ "client": map[string]interface{}{ - "local": gconn.LocalAddr().String(), - "remote": gconn.RemoteAddr().String(), + "local": LocalAddr(gconn), + "remote": RemoteAddr(gconn), }, "error": "", } @@ -208,8 +208,8 @@ func (s *Server) OnClose(gconn gnet.Conn, err error) gnet.Action { // Run the OnClosed hooks. data = map[string]interface{}{ "client": map[string]interface{}{ - "local": gconn.LocalAddr().String(), - "remote": gconn.RemoteAddr().String(), + "local": LocalAddr(gconn), + "remote": RemoteAddr(gconn), }, "error": "", } @@ -240,8 +240,8 @@ func (s *Server) OnTraffic(gconn gnet.Conn) gnet.Action { // Run the OnTraffic hooks. onTrafficData := map[string]interface{}{ "client": map[string]interface{}{ - "local": gconn.LocalAddr().String(), - "remote": gconn.RemoteAddr().String(), + "local": LocalAddr(gconn), + "remote": RemoteAddr(gconn), }, } _, err := s.pluginRegistry.Run( diff --git a/network/utils.go b/network/utils.go index 8797e109..0d4a3dac 100644 --- a/network/utils.go +++ b/network/utils.go @@ -63,6 +63,10 @@ func trafficData( } data := map[string]interface{}{ + "client": map[string]interface{}{ + "local": LocalAddr(gconn), + "remote": RemoteAddr(gconn), + }, "server": map[string]interface{}{ "local": client.LocalAddr(), "remote": client.RemoteAddr(), @@ -70,28 +74,6 @@ func trafficData( "error": "", } - //nolint:nestif - if gconn != nil { - data["client"] = map[string]interface{}{} - if gconn.LocalAddr() != nil { - if client, ok := data["client"].(map[string]interface{}); ok { - client["local"] = gconn.LocalAddr().String() - } - } - if gconn.RemoteAddr() != nil { - if client, ok := data["client"].(map[string]interface{}); ok { - client["remote"] = gconn.RemoteAddr().String() - } - } - } - - if client != nil { - data["server"] = map[string]interface{}{ - "local": client.LocalAddr(), - "remote": client.RemoteAddr(), - } - } - for _, field := range fields { // field.Value will be converted to base64-encoded string. data[field.Name] = field.Value @@ -149,3 +131,19 @@ func IsConnTimedOut(err *gerr.GatewayDError) bool { func IsConnClosed(received int, err *gerr.GatewayDError) bool { return received == 0 && err != nil && err.Unwrap() != nil && errors.Is(err.Unwrap(), io.EOF) } + +// LocalAddr returns the local address of the connection. +func LocalAddr(gconn gnet.Conn) string { + if gconn != nil && gconn.LocalAddr() != nil { + return gconn.LocalAddr().String() + } + return "" +} + +// RemoteAddr returns the remote address of the connection. +func RemoteAddr(gconn gnet.Conn) string { + if gconn != nil && gconn.RemoteAddr() != nil { + return gconn.RemoteAddr().String() + } + return "" +} From 1f8ba289c01558ebfb0514d6d4ad0f2018fc3718 Mon Sep 17 00:00:00 2001 From: Mostafa Moradian Date: Fri, 9 Jun 2023 18:57:23 +0200 Subject: [PATCH 3/3] Add ticket as comment in hotspots --- network/proxy.go | 4 ++++ network/server.go | 2 ++ 2 files changed, 6 insertions(+) diff --git a/network/proxy.go b/network/proxy.go index ac7a042d..323d9257 100644 --- a/network/proxy.go +++ b/network/proxy.go @@ -352,6 +352,8 @@ func (pr *Proxy) PassThrough(gconn gnet.Conn) *gerr.GatewayDError { // The connection to the server is closed, so we MUST reconnect, // otherwise the client will be stuck. + // TODO: Fix bug in handling connection close + // See: https://github.com/gatewayd-io/gatewayd/issues/219 if IsConnClosed(received, err) || IsConnTimedOut(err) { pr.logger.Debug().Fields( map[string]interface{}{ @@ -371,6 +373,8 @@ func (pr *Proxy) PassThrough(gconn gnet.Conn) *gerr.GatewayDError { } // If the response is empty, don't send anything, instead just close the ingress connection. + // TODO: Fix bug in handling connection close + // See: https://github.com/gatewayd-io/gatewayd/issues/219 if received == 0 { pr.logger.Debug().Fields( map[string]interface{}{ diff --git a/network/server.go b/network/server.go index 310ccc7c..bfa7a48d 100644 --- a/network/server.go +++ b/network/server.go @@ -266,6 +266,8 @@ func (s *Server) OnTraffic(gconn gnet.Conn) gnet.Action { errors.Is(err, gerr.ErrClientReceiveFailed), errors.Is(err, gerr.ErrHookTerminatedConnection), errors.Is(err.Unwrap(), io.EOF): + // TODO: Fix bug in handling connection close + // See: https://github.com/gatewayd-io/gatewayd/issues/219 return gnet.Close } }