I've accidentally spotted a bug when parts of a message from previous connection go to the next message.

I have a basic server with client. I have removed all the error handling to avoid bloating the examples too much.

Also I've replaced some Printf's with time.Sleep since I just don't have a chance to break the connection in time to reproduce the bug because it reads the data too fast.

The "package" is a simple structure, where the first 4 bytes is the length and then goes the content.

Client code:

package main

import (
    "encoding/binary"
    "fmt"
    "net"
)

func main() {
    conn, _ := net.Dial("tcp", "0.0.0.0:8081")
    defer conn.Close()

    str := "msadsakdjsajdklsajdklsajdk"

    // Creating a package
    buf := make([]byte, len(str) 4)
    copy(buf[4:], str)
    binary.LittleEndian.PutUint32(buf[:4], uint32(len(str)))

    for {
        _, err := conn.Write(buf)
        if err != nil {
            fmt.Println(err)
            return
        }
    }
}

Server code:

package main

import (
    "encoding/binary"
    "fmt"
    "net"
    "sync"
    "time"
)

func ReadConnection(conn net.Conn, buf []byte) (err error) {
    maxLen := cap(buf)
    readSize := 0
    for readSize < maxLen {
        // instead of Printf
        time.Sleep(time.Nanosecond * 10)

        readN, err := conn.Read(buf[readSize:])
        if err != nil {
            return err
        }
        readSize  = readN
    }
    return nil
}

func handleConnection(conn net.Conn, waitGroup *sync.WaitGroup) {
    waitGroup.Add(1)
    defer conn.Close()
    defer waitGroup.Done()

    fmt.Printf("Serving %s\n", conn.RemoteAddr().String())

    var packageSize int32 = 0
    int32Buf := make([]byte, 4)

    for {
        // read the length
        conn.Read(int32Buf)
        packageSize = int32(binary.LittleEndian.Uint32(int32Buf))

        // assuming the length should be 26
        if packageSize > 26 {
            fmt.Println("Package size error")
            return
        }

        // read the content
        packageBuf := make([]byte, packageSize)
        if err := ReadConnection(conn, packageBuf); err != nil {
            fmt.Printf("ERR: %s\n", err)
            return
        }

        // instead of Printf
        time.Sleep(time.Nanosecond * 100)
    }
}

func main() {
    //establish connection
    listener, _ := net.Listen("tcp", "0.0.0.0:8081")
    defer listener.Close()

    waitGroup := sync.WaitGroup{}
    for {
        conn, err := listener.Accept()
        if err != nil {
            break
        }

        go handleConnection(conn, &waitGroup)
    }

    waitGroup.Wait()
}

So for some reason, int32Buf receives the last 2 bytes from a previous message (d, k) and the first 2 bytes of the length, resulting in [107, 100, 26, 0] bytes slice, when it should be [26, 0, 0, 0]. And of course, the rest of the data contains remaining two zeroes:

CodePudding user response：

     conn.Read(int32Buf)

You need to check the return value of conn.Read and compare it against your expectations. You are assuming in your code that conn.Read will always completely fill the given buffer of 4 bytes.

This assumption is wrong, i.e. it might actually read less data. Specifically it might read only 2 bytes in which case you'll end up with \x1a\x00\x00\x00 in your buffer which still translates to a message length of 26. Only, the first 2 bytes of the message will actually be the last 2 bytes of the length which were not included in the last read. This means after reading the 26 bytes it will not have read the full message. 2 bytes are legt and will be included into the next message - this is what you observed.

To be sure that the exact size of the buffer is read check the return values of conn.Read or use io.ReadFull. After you've done this it works as expected (from the comment):

Ok, now it works perfect

So why does this happened only in context of a new connection? Maybe because the additional load due to another connection changed the behavior slightly but significantly enough. Still, these are not the data read from a different connection but data from the current one contrary to the description in the question. This could be easily checked by using different messages with different clients.