Fix packet parsing

With the old implementation we skipped one byte of the body and did not read
an empty body.
Not being able to read an empty body was not the only problem with the
old implementation. It could also happen that for what ever reason an
null byte was encoded inside of it and with the old implementation we
would stop at this null and ignore the rest.

README.md

@@ -5,35 +5,37 @@ This is a fork from [james4k/rcon](https://github.com/james4k/rcon) with the sup
 
 ## Usage
 ```golang
-// Espablish a connection.
+// Establish a connection.
 remoteConsole, err := rcon.Dial("127.0.0.1", "password")
 if err != nil {
-    fmt.Println(err)
+    log.Fatal(err)
 }
+// Close the connection at the end to free the used resources.
+defer remoteConsole.Close()
 
 // Send a command.
 requestID, err := remoteConsole.Write("command")
 if err != nil {
-    fmt.Println(err)
+    log.Fatal(err)
 }
 
-// Read the response
+// Read the response.
 response, responseID, err := remoteConsole.Read()
 if err != nil {
-    fmt.Println(err)
+    log.Fatal(err)
 }
 if requestID != responseID {
-    fmt.Println("response id doesn't match the request id!")
+    log.Fatal("response id doesn't match the request id!")
 }
 
 fmt.Println(response)
 ```
 
 ## License
-This lib is licesed under the [MIT License](LICENSE)
+This lib is licensed under the [MIT License](LICENSE).
 
 ## Contributors
-If you should encaunter a bug or a missing feature dont hessitate to open an issue or even submit a pull-request.
+If you should encounter a bug or a missing feature don't hesitate to open an issue or even submit a pull-request.
 
 
 Special thx to [nhh](https://github.com/nhh) and [dnltinney](https://github.com/dnltinney) for the great help debugging this lib.

rcon.go

@@ -1,17 +1,23 @@
+/*
+A Go written library for the RCON Protocol from Valve.
+
+Information to the protocol can be found under:
+https://developer.valvesoftware.com/wiki/Source_RCON_Protocol
+
+This is a fork from https://github.com/james4k/rcon with the support for go
+modules and with a rework of the original implementation for better readability.
+*/
 package rcon
 
 import (
 	"bytes"
 	"encoding/binary"
 	"errors"
-	"io"
 	"net"
 	"sync"
 	"time"
 )
 
-// Information to the protocol can be found under: https://developer.valvesoftware.com/wiki/Source_RCON_Protocol
-
 const (
 	typeAuth          = 3
 	typeExecCommand   = 2
@@ -78,13 +84,16 @@ func Dial(host, password string) (*RemoteConsole, error) {
 		return nil, err
 	}
 
-	r := &RemoteConsole{conn: conn, readBuff: make([]byte, maxPackageSize+fieldPackageSize)}
-	r.auth(password, timeout)
+	remoteConsole := &RemoteConsole{
+		conn:     conn,
+		readBuff: make([]byte, maxPackageSize+fieldPackageSize),
+	}
+	remoteConsole.auth(password, timeout)
 	if err != nil {
 		return nil, err
 	}
 
-	return r, nil
+	return remoteConsole, nil
 }
 
 // LocalAddr returns the local network address.
@@ -182,15 +191,14 @@ func (r *RemoteConsole) writeCmd(reqID, pkgType int32, cmd string) error {
 	// request id
 	binary.Write(buffer, binary.LittleEndian, int32(reqID))
 
-	// auth cmd
+	// type of the package
 	binary.Write(buffer, binary.LittleEndian, int32(pkgType))
 
-	// string (null terminated)
+	// body
 	buffer.WriteString(cmd)
-	binary.Write(buffer, binary.LittleEndian, byte(0))
 
-	// string 2 (null terminated)
-	// we don't have a use for string 2
+	// double null termination
+	binary.Write(buffer, binary.LittleEndian, byte(0))
 	binary.Write(buffer, binary.LittleEndian, byte(0))
 
 	r.conn.SetWriteDeadline(time.Now().Add(10 * time.Second))
@@ -287,17 +295,22 @@ func (r *RemoteConsole) readResponsePackage(totalPackageSize, readBytes int) (in
 
 func (r *RemoteConsole) readResponseData(data []byte) (int, int, []byte, error) {
 	var requestID, responseType int32
-	var response []byte
 	buffer := bytes.NewBuffer(data)
-	binary.Read(buffer, binary.LittleEndian, &requestID)
+
+	err := binary.Read(buffer, binary.LittleEndian, &requestID)
+	if err != nil {
+		return 0, 0, []byte{}, err
+	}
+
 	binary.Read(buffer, binary.LittleEndian, &responseType)
-	response, err := buffer.ReadBytes(byte(0))
-	if err != nil && err != io.EOF {
-		return 0, 0, nil, err
+	if err != nil {
+		return 0, 0, []byte{}, err
 	}
-	if err == nil {
-		// if we didn't hit EOF, we have a null byte to remove
-		response = response[:len(response)-1]
-	}
-	return int(responseType), int(requestID), response, nil
+
+	// the rest of the buffer is the body.
+	body := buffer.Bytes()
+	// remove the to null terminations
+	body = body[:len(body)-2]
+
+	return int(responseType), int(requestID), body, nil
 }

rcon_test.go

@@ -7,6 +7,8 @@ import (
 	"testing"
 )
 
+const password = "password"
+
 func startTestServer(fn func(net.Conn, *bytes.Buffer)) (string, error) {
 	listener, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {
@@ -36,7 +38,7 @@ func startTestServer(fn func(net.Conn, *bytes.Buffer)) (string, error) {
 		if err != nil {
 			return
 		}
-		if string(str[:len(str)-1]) != "blerg" {
+		if string(str[:len(str)-1]) != password {
 			requestID = -1
 		}
 
@@ -96,7 +98,7 @@ func TestAuth(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	rc, err := Dial(addr, "blerg")
+	rc, err := Dial(addr, password)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -118,7 +120,7 @@ func TestMultipacket(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		rc, err := Dial(addr, "blerg")
+		rc, err := Dial(addr, password)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -144,7 +146,7 @@ func TestMultipacket(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		rc, err := Dial(addr, "blerg")
+		rc, err := Dial(addr, password)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -172,7 +174,7 @@ func TestMultipacket(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		rc, err := Dial(addr, "blerg")
+		rc, err := Dial(addr, password)
 		if err != nil {
 			t.Fatal(err)
 		}