diff --git a/wlms/server.go b/wlms/server.go index 0313640..37628d8 100644 --- a/wlms/server.go +++ b/wlms/server.go @@ -548,7 +548,9 @@ func CreateServerUsing(acceptedConnections chan ReadWriteCloserWithIp, db UserDb } func (s *Server) mainLoop() { - defer s.relay.CloseConnection() + if s.relay != nil { + defer s.relay.CloseConnection() + } // Remove (non-IRC) clients and games that are older than this time. // Normally, I expect this never to remove anything, except for // when bugs / unexpected states make a client/game survive. diff --git a/wlnr/client.go b/wlnr/client.go index e82954b..5073611 100644 --- a/wlnr/client.go +++ b/wlnr/client.go @@ -8,8 +8,6 @@ import ( "time" ) -const PING_INTERVAL_S = 90 - // Structure to bundle the TCP connection with its packet buffer type Client struct { // The TCP connection to the client @@ -57,7 +55,7 @@ func New(conn net.Conn) *Client { id: 0, reader: bufio.NewReader(conn), chan_out: make(chan *Command), - pingTimer: time.NewTimer(time.Second * 1), // Do the next ping now + pingTimer: time.NewTimer(time.Second * 1), waitingForPong: false, lastSendPingSeq: 0, timeLastPing: time.Now(), @@ -106,11 +104,6 @@ func (c *Client) ReadPacket() ([]byte, error) { packet[0] = length_bytes[0] packet[1] = length_bytes[1] _, error = io.ReadFull(c.reader, packet[2:]) - // TODO(Notabilis): Think about this (and similar places). The client might be able - // to keep the server waiting here. Actually, he can simply keep the connection - // idling anyway. Is this a problem? Might be a possibility for DoS. - // Is there a ping in the GameHost code? Won't help before a game is assigned - // to the client, though. So probably add some fast disconnect on idle. return packet, error } @@ -149,14 +142,24 @@ func (c *Client) pingLoop() { cmd := NewCommand(kPing) cmd.AppendUInt(c.lastSendPingSeq) c.SendCommand(cmd) - c.pingTimer.Reset(time.Second * PING_INTERVAL_S) + c.pingTimer.Reset(time.Second * 1) } else { - // Bad luck: We got no response so disconnect client - // In the case of the game host this also takes down the game - // by closing the socket -> game will notice it and abort - log.Printf("Timeout of client (id=%v), disconnecting", c.id) - c.Disconnect("TIMEOUT") - break + // If it is over 90 seconds since the last received PONG, + // the client is hanging for too long and we disconnect it. + // Normally we should receive a PONG every second, but for build20 + // clients this might be delayed when loading maps + if time.Now().After(c.timeLastPong.Add(time.Second * 90)) { + // Bad luck: We got no response for too long so disconnect client + // In the case of the game host this also takes down the game + // by closing the socket -> game will notice it and abort + log.Printf("Timeout of client (id=%v), disconnecting", c.id) + c.Disconnect("TIMEOUT") + break + } else { + // We are using TCP, so Pings can't get lost. + // Just wait for some more time + c.pingTimer.Reset(time.Second * 1) + } } } } @@ -164,11 +167,8 @@ func (c *Client) pingLoop() { func (c *Client) HandlePong(seq uint8) { c.waitingForPong = false if seq != c.lastSendPingSeq { - // Well, actually the sequence numbers are not that important. - // The client will be disconnected when he fails to respond in time, - // so there should never be two pings open at the same time. - // Could become important if we add, e.g., fast pings to check - // whether a certain client is active. + // Well, actually the sequence numbers are not that important, + // as there should never be two pings open at the same time. // Use the sequence number anyway so we don't mess up our // measurements when we get a pong too much return diff --git a/wlnr/command_codes.go b/wlnr/command_codes.go index c5d2c7c..e33cc19 100644 --- a/wlnr/command_codes.go +++ b/wlnr/command_codes.go @@ -1,7 +1,10 @@ package main const ( - kRelayProtocolVersion uint8 = 1 + kRelayProtocolVersion uint8 = 2 + // Build 20 contains a bug that the password is sent twice. + // Apart from that, the relay protocol versions are the same + kRelayProtocolVersionBuild20 uint8 = 1 // The commands used in the protocol // The names match the names in the Widelands sources diff --git a/wlnr/game.go b/wlnr/game.go index 5c69c7e..e1cc366 100644 --- a/wlnr/game.go +++ b/wlnr/game.go @@ -230,6 +230,9 @@ func (game *Game) handleClientMessages(client *Client) { game.handlePong(client) case kRoundTripTimeRequest: game.sendRTTs(client) + default: + // Unexpected command, exit + game.DisconnectClient(client, "PROTOCOL_VIOLATION") } } @@ -289,6 +292,19 @@ func (game *Game) handleHostMessages() { game.handlePong(game.host) case kRoundTripTimeRequest: game.sendRTTs(game.host) + case kDisconnectClient: + id, err := game.host.ReadUint8() + if err != nil || id <= 1 { + game.DisconnectClient(game.host, "PROTOCOL_VIOLATION") + return + } + client := game.getClient(id) + if client != nil { + game.DisconnectClient(client, "NORMAL") + } + default: + // Unexpected command, exit + game.DisconnectClient(game.host, "PROTOCOL_VIOLATION") } } } diff --git a/wlnr/server.go b/wlnr/server.go index f3e985d..4191c35 100644 --- a/wlnr/server.go +++ b/wlnr/server.go @@ -161,7 +161,7 @@ func (s *Server) dealWithNewConnection(client *Client) { client.Disconnect("PROTOCOL_VIOLATION") return } - if version != kRelayProtocolVersion { + if version != kRelayProtocolVersion && version != kRelayProtocolVersionBuild20 { client.Disconnect("WRONG_VERSION") return } @@ -176,6 +176,14 @@ func (s *Server) dealWithNewConnection(client *Client) { client.Disconnect("PROTOCOL_VIOLATION") return } + + if version == kRelayProtocolVersionBuild20 && password != "client" { + // Build 20 contains a bug that the password is sent twice by the host. + // Apart from that, the relay protocol versions are the same. + // Receive and discard the useless second password + client.ReadString() + } + // The game will handle the client for e := s.games.Front(); e != nil; e = e.Next() { game := e.Value.(*Game)