Ping fixes

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.

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,26 +142,33 @@ 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)
+			}
 		}
 	}
 }
 
 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

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

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")
 		}
 	}
 }

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)