From ab16c7d82c8381a4df80fef4b2fe81e7ff23dad8 Mon Sep 17 00:00:00 2001 From: Jeff Halter <868228+jhalter@users.noreply.github.com> Date: Mon, 24 Nov 2025 16:52:37 -0800 Subject: Fix tracker server list parsing for batched responses The tracker protocol splits large server lists into batches, with each batch preceded by a ServerInfoHeader. Previously, GetListing only read the initial header and failed when encountering subsequent headers mid-stream, causing the scanner to misinterpret header bytes as server record data. Changes: - Refactored GetListing to use bufio.Reader instead of bufio.Scanner to handle heterogeneous data (headers and server records) - Added readServerRecord helper function for sequential reading - Renamed ServerInfoHeader.SrvCountDup to BatchSize for clarity - Added comprehensive documentation explaining the batching protocol - Removed unused serverScanner function and tests - Added table tests covering multiple batch scenarios The BatchSize field indicates the number of servers in the current batch, while SrvCount indicates the total across all batches. --- hotline/tracker.go | 114 +++++++++------ hotline/tracker_test.go | 375 ++++++++++++++++++++++++++++++------------------ 2 files changed, 307 insertions(+), 182 deletions(-) (limited to 'hotline') diff --git a/hotline/tracker.go b/hotline/tracker.go index edd973d..bfba69d 100644 --- a/hotline/tracker.go +++ b/hotline/tracker.go @@ -91,11 +91,22 @@ type TrackerHeader struct { Version [2]byte // Old protocol (1) or new (2) } +// ServerInfoHeader represents a batch header in the tracker response. +// The tracker protocol splits large server lists into batches, with each batch +// preceded by its own ServerInfoHeader. The first header indicates the total +// number of servers across all batches, and each header (including the first) +// indicates how many servers are in the current batch. +// +// Example flow for 106 servers split into batches: +// 1. First ServerInfoHeader: SrvCount=106, BatchSize=97 +// 2. Read 97 ServerRecords +// 3. Second ServerInfoHeader: SrvCount=106, BatchSize=9 +// 4. Read 9 ServerRecords (total: 106) type ServerInfoHeader struct { MsgType [2]byte // Always has value of 1 MsgDataSize [2]byte // Remaining size of request - SrvCount [2]byte // Number of servers in the server list - SrvCountDup [2]byte // Same as previous field ¯\_(ツ)_/¯ + SrvCount [2]byte // Total number of servers across all batches + BatchSize [2]byte // Number of servers in the current batch } // ServerRecord is a tracker listing for a single server @@ -128,79 +139,92 @@ func GetListing(conn io.ReadWriteCloser) ([]ServerRecord, error) { return nil, err } + // Use a buffered reader so we can read both headers and server records from the same buffer + reader := bufio.NewReader(conn) + var info ServerInfoHeader - if err := binary.Read(conn, binary.BigEndian, &info); err != nil { + if err := binary.Read(reader, binary.BigEndian, &info); err != nil { return nil, err } totalSrv := int(binary.BigEndian.Uint16(info.SrvCount[:])) + batchSize := int(binary.BigEndian.Uint16(info.BatchSize[:])) - scanner := bufio.NewScanner(conn) - scanner.Split(serverScanner) + servers := make([]ServerRecord, 0, totalSrv) + serversInCurrentBatch := 0 - var servers []ServerRecord - for { - scanner.Scan() + for len(servers) < totalSrv { + // Check if we've read all servers in the current batch + if serversInCurrentBatch == batchSize { + // Read the next ServerInfoHeader for the next batch from the buffered reader + if err := binary.Read(reader, binary.BigEndian, &info); err != nil { + return nil, fmt.Errorf("failed to read next ServerInfoHeader after %d servers: %w", len(servers), err) + } - // Make a new []byte slice and copy the scanner bytes to it. This is critical as the - // scanner re-uses the buffer for subsequent scans. - buf := make([]byte, len(scanner.Bytes())) - copy(buf, scanner.Bytes()) + batchSize = int(binary.BigEndian.Uint16(info.BatchSize[:])) + serversInCurrentBatch = 0 + } - var srv ServerRecord - _, err = srv.Write(buf) + // Read a server record using our helper function + srv, err := readServerRecord(reader) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to read server record %d: %w", len(servers)+1, err) } servers = append(servers, srv) - if len(servers) == totalSrv { - break - } + serversInCurrentBatch++ } return servers, nil } -// serverScanner implements bufio.SplitFunc for parsing the tracker list into ServerRecords tokens -// Example payload: -// 00000000 18 05 30 63 15 7c 00 02 00 00 10 54 68 65 20 4d |..0c.|.....The M| -// 00000010 6f 62 69 75 73 20 53 74 72 69 70 40 48 6f 6d 65 |obius Strip@Home| -// 00000020 20 6f 66 20 74 68 65 20 4d 6f 62 69 75 73 20 48 | of the Mobius H| -// 00000030 6f 74 6c 69 6e 65 20 73 65 72 76 65 72 20 61 6e |otline server an| -// 00000040 64 20 63 6c 69 65 6e 74 20 7c 20 54 52 54 50 48 |d client | TRTPH| -// 00000050 4f 54 4c 2e 63 6f 6d 3a 35 35 30 30 2d 4f 3a b2 |OTL.com:5500-O:.| -// 00000060 15 7c 00 00 00 00 08 53 65 6e 65 63 74 75 73 20 |.|.....Senectus | -func serverScanner(data []byte, _ bool) (advance int, token []byte, err error) { - // The name length field is the 11th byte of the server record. If we don't have that many bytes, - // return nil token so the Scanner reads more data and continues scanning. - if len(data) < 10 { - return 0, nil, nil +// readServerRecord reads a single ServerRecord from the reader +func readServerRecord(reader *bufio.Reader) (ServerRecord, error) { + var srv ServerRecord + + // Read fixed header: IP (4) + Port (2) + NumUsers (2) + Unused (2) = 10 bytes + header := make([]byte, 10) + if _, err := io.ReadFull(reader, header); err != nil { + return srv, fmt.Errorf("failed to read server header: %w", err) } - // A server entry has two variable length fields: the name and description. - // To get the token length, we first need the name length from the 10th byte - nameLen := int(data[10]) + copy(srv.IPAddr[:], header[0:4]) + copy(srv.Port[:], header[4:6]) + copy(srv.NumUsers[:], header[6:8]) + copy(srv.Unused[:], header[8:10]) - // The description length field is at the 12th + nameLen byte of the server record. - // If we don't have that many bytes, return nil token so the Scanner reads more data and continues scanning. - if len(data) < 11+nameLen { - return 0, nil, nil + // Read name size + nameSizeByte, err := reader.ReadByte() + if err != nil { + return srv, fmt.Errorf("failed to read name size: %w", err) } + srv.NameSize = nameSizeByte - // Next we need the description length from the 11+nameLen byte: - descLen := int(data[11+nameLen]) + // Read name + srv.Name = make([]byte, srv.NameSize) + if _, err := io.ReadFull(reader, srv.Name); err != nil { + return srv, fmt.Errorf("failed to read name: %w", err) + } + + // Read description size + descSizeByte, err := reader.ReadByte() + if err != nil { + return srv, fmt.Errorf("failed to read description size: %w", err) + } + srv.DescriptionSize = descSizeByte - if len(data) < 12+nameLen+descLen { - return 0, nil, nil + // Read description + srv.Description = make([]byte, srv.DescriptionSize) + if _, err := io.ReadFull(reader, srv.Description); err != nil { + return srv, fmt.Errorf("failed to read description: %w", err) } - return 12 + nameLen + descLen, data[0 : 12+nameLen+descLen], nil + return srv, nil } // Write implements io.Writer for ServerRecord func (s *ServerRecord) Write(b []byte) (n int, err error) { - if len(b) < 13 { + if len(b) < 12 { return 0, errors.New("too few bytes") } copy(s.IPAddr[:], b[0:4]) diff --git a/hotline/tracker_test.go b/hotline/tracker_test.go index 5457e47..8295582 100644 --- a/hotline/tracker_test.go +++ b/hotline/tracker_test.go @@ -2,11 +2,11 @@ package hotline import ( "bytes" - "fmt" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "io" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestTrackerRegistration_Payload(t *testing.T) { @@ -62,138 +62,6 @@ func TestTrackerRegistration_Payload(t *testing.T) { } } -func Test_serverScanner(t *testing.T) { - type args struct { - data []byte - atEOF bool - } - tests := []struct { - name string - args args - wantAdvance int - wantToken []byte - wantErr assert.ErrorAssertionFunc - }{ - { - name: "when a full server entry is provided", - args: args{ - data: []byte{ - 0x18, 0x05, 0x30, 0x63, // IP Addr - 0x15, 0x7c, // Port - 0x00, 0x02, // UserCount - 0x00, 0x00, // ?? - 0x03, // Name Len - 0x54, 0x68, 0x65, // Name - 0x03, // Desc Len - 0x54, 0x54, 0x54, // Description - }, - atEOF: false, - }, - wantAdvance: 18, - wantToken: []byte{ - 0x18, 0x05, 0x30, 0x63, // IP Addr - 0x15, 0x7c, // Port - 0x00, 0x02, // UserCount - 0x00, 0x00, // ?? - 0x03, // Name Len - 0x54, 0x68, 0x65, // Name - 0x03, // Desc Len - 0x54, 0x54, 0x54, // Description - }, - wantErr: assert.NoError, - }, - { - name: "when extra bytes are provided", - args: args{ - data: []byte{ - 0x18, 0x05, 0x30, 0x63, // IP Addr - 0x15, 0x7c, // Port - 0x00, 0x02, // UserCount - 0x00, 0x00, // ?? - 0x03, // Name Len - 0x54, 0x68, 0x65, // Name - 0x03, // Desc Len - 0x54, 0x54, 0x54, // Description - 0x54, 0x54, 0x54, 0x54, 0x54, 0x54, 0x54, 0x54, 0x54, - }, - atEOF: false, - }, - wantAdvance: 18, - wantToken: []byte{ - 0x18, 0x05, 0x30, 0x63, // IP Addr - 0x15, 0x7c, // Port - 0x00, 0x02, // UserCount - 0x00, 0x00, // ?? - 0x03, // Name Len - 0x54, 0x68, 0x65, // Name - 0x03, // Desc Len - 0x54, 0x54, 0x54, // Description - }, - wantErr: assert.NoError, - }, - { - name: "when insufficient bytes are provided", - args: args{ - data: []byte{ - 0, 0, - }, - atEOF: false, - }, - wantAdvance: 0, - wantToken: []byte(nil), - wantErr: assert.NoError, - }, - { - name: "when nameLen exceeds provided data", - args: args{ - data: []byte{ - 0x18, 0x05, 0x30, 0x63, // IP Addr - 0x15, 0x7c, // Port - 0x00, 0x02, // UserCount - 0x00, 0x00, // ?? - 0xff, // Name Len - 0x54, 0x68, 0x65, // Name - 0x03, // Desc Len - 0x54, 0x54, 0x54, // Description - }, - atEOF: false, - }, - wantAdvance: 0, - wantToken: []byte(nil), - wantErr: assert.NoError, - }, - { - name: "when description len exceeds provided data", - args: args{ - data: []byte{ - 0x18, 0x05, 0x30, 0x63, // IP Addr - 0x15, 0x7c, // Port - 0x00, 0x02, // UserCount - 0x00, 0x00, // ?? - 0x03, // Name Len - 0x54, 0x68, 0x65, // Name - 0xff, // Desc Len - 0x54, 0x54, 0x54, // Description - }, - atEOF: false, - }, - wantAdvance: 0, - wantToken: []byte(nil), - wantErr: assert.NoError, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gotAdvance, gotToken, err := serverScanner(tt.args.data, tt.args.atEOF) - if !tt.wantErr(t, err, fmt.Sprintf("serverScanner(%v, %v)", tt.args.data, tt.args.atEOF)) { - return - } - assert.Equalf(t, tt.wantAdvance, gotAdvance, "serverScanner(%v, %v)", tt.args.data, tt.args.atEOF) - assert.Equalf(t, tt.wantToken, gotToken, "serverScanner(%v, %v)", tt.args.data, tt.args.atEOF) - }) - } -} - type mockConn struct { readBuffer *bytes.Buffer writeBuffer *bytes.Buffer @@ -231,7 +99,7 @@ func TestGetListing(t *testing.T) { 0x00, 0x01, // MsgType (1) 0x00, 0x14, // MsgDataSize (20) 0x00, 0x02, // SrvCount (2) - 0x00, 0x02, // SrvCountDup (2) + 0x00, 0x02, // BatchSize (2) // ServerRecord 1 192, 168, 1, 1, // IP address 0x1F, 0x90, // Port 8080 @@ -324,7 +192,7 @@ func TestGetListing(t *testing.T) { 0x00, 0x01, // MsgType (1) 0x00, 0x14, // MsgDataSize (20) 0x00, 0x01, // SrvCount (1) - 0x00, 0x01, // SrvCountDup (1) + 0x00, 0x01, // BatchSize (1) // incomplete ServerRecord to cause scanner error 192, 168, 1, 1, }), @@ -333,6 +201,239 @@ func TestGetListing(t *testing.T) { wantErr: true, wantResult: nil, }, + { + name: "Multiple batches with ServerInfoHeaders", + mockConn: &mockConn{ + readBuffer: bytes.NewBuffer([]byte{ + // TrackerHeader + 0x48, 0x54, 0x52, 0x4B, // Protocol "HTRK" + 0x00, 0x01, // Version 1 + // First ServerInfoHeader + 0x00, 0x01, // MsgType (1) + 0x00, 0x14, // MsgDataSize (20) + 0x00, 0x03, // SrvCount (3 total) + 0x00, 0x02, // BatchSize (2 in first batch) + // ServerRecord 1 + 192, 168, 1, 1, // IP address + 0x1F, 0x90, // Port 8080 + 0x00, 0x0A, // NumUsers 10 + 0x00, 0x00, // Unused + 0x07, // NameSize + 'S', 'e', 'r', 'v', 'e', 'r', '1', // Name + 0x0C, // DescriptionSize + 'F', 'i', 'r', 's', 't', ' ', 'b', 'a', 't', 'c', 'h', '1', // Description + // ServerRecord 2 + 192, 168, 1, 2, // IP address + 0x1F, 0x91, // Port 8081 + 0x00, 0x14, // NumUsers 20 + 0x00, 0x00, // Unused + 0x07, // NameSize + 'S', 'e', 'r', 'v', 'e', 'r', '2', // Name + 0x0C, // DescriptionSize + 'F', 'i', 'r', 's', 't', ' ', 'b', 'a', 't', 'c', 'h', '2', // Description + // Second ServerInfoHeader (next batch) + 0x00, 0x01, // MsgType (1) + 0x00, 0x0A, // MsgDataSize (10) + 0x00, 0x03, // SrvCount (3 total - same) + 0x00, 0x01, // BatchSize (1 in second batch) + // ServerRecord 3 + 192, 168, 1, 3, // IP address + 0x1F, 0x92, // Port 8082 + 0x00, 0x1E, // NumUsers 30 + 0x00, 0x00, // Unused + 0x07, // NameSize + 'S', 'e', 'r', 'v', 'e', 'r', '3', // Name + 0x0D, // DescriptionSize + 'S', 'e', 'c', 'o', 'n', 'd', ' ', 'b', 'a', 't', 'c', 'h', '1', // Description + }), + writeBuffer: &bytes.Buffer{}, + }, + wantErr: false, + wantResult: []ServerRecord{ + { + IPAddr: [4]byte{192, 168, 1, 1}, + Port: [2]byte{0x1F, 0x90}, + NumUsers: [2]byte{0x00, 0x0A}, + Unused: [2]byte{0x00, 0x00}, + NameSize: 7, + Name: []byte("Server1"), + DescriptionSize: 12, + Description: []byte("First batch1"), + }, + { + IPAddr: [4]byte{192, 168, 1, 2}, + Port: [2]byte{0x1F, 0x91}, + NumUsers: [2]byte{0x00, 0x14}, + Unused: [2]byte{0x00, 0x00}, + NameSize: 7, + Name: []byte("Server2"), + DescriptionSize: 12, + Description: []byte("First batch2"), + }, + { + IPAddr: [4]byte{192, 168, 1, 3}, + Port: [2]byte{0x1F, 0x92}, + NumUsers: [2]byte{0x00, 0x1E}, + Unused: [2]byte{0x00, 0x00}, + NameSize: 7, + Name: []byte("Server3"), + DescriptionSize: 13, + Description: []byte("Second batch1"), + }, + }, + }, + { + name: "Three batches", + mockConn: &mockConn{ + readBuffer: bytes.NewBuffer([]byte{ + // TrackerHeader + 0x48, 0x54, 0x52, 0x4B, // Protocol "HTRK" + 0x00, 0x01, // Version 1 + // First ServerInfoHeader + 0x00, 0x01, // MsgType (1) + 0x00, 0x0A, // MsgDataSize + 0x00, 0x04, // SrvCount (4 total) + 0x00, 0x02, // BatchSize (2 in first batch) + // ServerRecord 1 + 192, 168, 1, 1, // IP + 0x15, 0x7c, // Port 5500 + 0x00, 0x01, // NumUsers 1 + 0x00, 0x00, // Unused + 0x01, // NameSize + 'A', // Name + 0x01, // DescriptionSize + '1', // Description + // ServerRecord 2 + 192, 168, 1, 2, // IP + 0x15, 0x7c, // Port 5500 + 0x00, 0x02, // NumUsers 2 + 0x00, 0x00, // Unused + 0x01, // NameSize + 'B', // Name + 0x01, // DescriptionSize + '2', // Description + // Second ServerInfoHeader + 0x00, 0x01, // MsgType (1) + 0x00, 0x0A, // MsgDataSize + 0x00, 0x04, // SrvCount (4 total) + 0x00, 0x01, // BatchSize (1 in second batch) + // ServerRecord 3 + 192, 168, 1, 3, // IP + 0x15, 0x7c, // Port 5500 + 0x00, 0x03, // NumUsers 3 + 0x00, 0x00, // Unused + 0x01, // NameSize + 'C', // Name + 0x01, // DescriptionSize + '3', // Description + // Third ServerInfoHeader + 0x00, 0x01, // MsgType (1) + 0x00, 0x0A, // MsgDataSize + 0x00, 0x04, // SrvCount (4 total) + 0x00, 0x01, // BatchSize (1 in third batch) + // ServerRecord 4 + 192, 168, 1, 4, // IP + 0x15, 0x7c, // Port 5500 + 0x00, 0x04, // NumUsers 4 + 0x00, 0x00, // Unused + 0x01, // NameSize + 'D', // Name + 0x01, // DescriptionSize + '4', // Description + }), + writeBuffer: &bytes.Buffer{}, + }, + wantErr: false, + wantResult: []ServerRecord{ + { + IPAddr: [4]byte{192, 168, 1, 1}, + Port: [2]byte{0x15, 0x7c}, + NumUsers: [2]byte{0x00, 0x01}, + Unused: [2]byte{0x00, 0x00}, + NameSize: 1, + Name: []byte("A"), + DescriptionSize: 1, + Description: []byte("1"), + }, + { + IPAddr: [4]byte{192, 168, 1, 2}, + Port: [2]byte{0x15, 0x7c}, + NumUsers: [2]byte{0x00, 0x02}, + Unused: [2]byte{0x00, 0x00}, + NameSize: 1, + Name: []byte("B"), + DescriptionSize: 1, + Description: []byte("2"), + }, + { + IPAddr: [4]byte{192, 168, 1, 3}, + Port: [2]byte{0x15, 0x7c}, + NumUsers: [2]byte{0x00, 0x03}, + Unused: [2]byte{0x00, 0x00}, + NameSize: 1, + Name: []byte("C"), + DescriptionSize: 1, + Description: []byte("3"), + }, + { + IPAddr: [4]byte{192, 168, 1, 4}, + Port: [2]byte{0x15, 0x7c}, + NumUsers: [2]byte{0x00, 0x04}, + Unused: [2]byte{0x00, 0x00}, + NameSize: 1, + Name: []byte("D"), + DescriptionSize: 1, + Description: []byte("4"), + }, + }, + }, + { + name: "Error reading second ServerInfoHeader", + mockConn: &mockConn{ + readBuffer: bytes.NewBuffer([]byte{ + // TrackerHeader + 0x48, 0x54, 0x52, 0x4B, // Protocol "HTRK" + 0x00, 0x01, // Version 1 + // First ServerInfoHeader + 0x00, 0x01, // MsgType (1) + 0x00, 0x0A, // MsgDataSize + 0x00, 0x02, // SrvCount (2 total) + 0x00, 0x01, // BatchSize (1 in first batch) + // ServerRecord 1 + 192, 168, 1, 1, // IP + 0x15, 0x7c, // Port 5500 + 0x00, 0x01, // NumUsers 1 + 0x00, 0x00, // Unused + 0x01, // NameSize + 'A', // Name + 0x01, // DescriptionSize + '1', // Description + // Incomplete second ServerInfoHeader + 0x00, 0x01, // MsgType only + }), + writeBuffer: &bytes.Buffer{}, + }, + wantErr: true, + wantResult: nil, + }, + { + name: "Empty server list", + mockConn: &mockConn{ + readBuffer: bytes.NewBuffer([]byte{ + // TrackerHeader + 0x48, 0x54, 0x52, 0x4B, // Protocol "HTRK" + 0x00, 0x01, // Version 1 + // ServerInfoHeader with 0 servers + 0x00, 0x01, // MsgType (1) + 0x00, 0x00, // MsgDataSize (0) + 0x00, 0x00, // SrvCount (0) + 0x00, 0x00, // BatchSize (0) + }), + writeBuffer: &bytes.Buffer{}, + }, + wantErr: false, + wantResult: []ServerRecord{}, + }, } for _, tt := range tests { -- cgit