hypebeast / go-osc

Open Sound Control (OSC) library for Golang. Implemented in pure Go.
MIT License
202 stars 46 forks source link

Problems parsing binary blobs #62

Open mhite opened 1 year ago

mhite commented 1 year ago

I am hitting an interesting error when parsing a binary blob.

Here's the code of interest:

func readBlob(reader *bufio.Reader) ([]byte, int, error) {
    // First, get the length
    var blobLen int32
    if err := binary.Read(reader, binary.BigEndian, &blobLen); err != nil {
        return nil, 0, err
    }
    n := 4 + int(blobLen)

    if blobLen < 1 || blobLen > int32(reader.Buffered()) {
        return nil, 0, fmt.Errorf("readBlob: invalid blob length %d", blobLen)
    }

    // Read the data
    blob := make([]byte, blobLen)
    if _, err := reader.Read(blob); err != nil {
        return nil, 0, err
    }

    // Remove the padding bytes
    numPadBytes := padBytesNeeded(int(blobLen))
    if numPadBytes > 0 {
        n += numPadBytes
        dummy := make([]byte, numPadBytes)
        if _, err := reader.Read(dummy); err != nil {
            return nil, 0, err
        }
    }

    return blob, n, nil
}

Specifically:

    if blobLen < 1 || blobLen > int32(reader.Buffered()) {
        return nil, 0, fmt.Errorf("readBlob: invalid blob length %d", blobLen)
    }

I am wondering reader.Buffered() is really returning the full length as expected.

For context, here's some debug logs I have from my application.

2023/08/04 11:09:10 Couldn't parse token
2023/08/04 11:09:10 err = readBlob: invalid blob length 11520
2023/08/04 11:09:10 token: ([]uint8) (len=11596 cap=16384) {
 00000000  23 62 75 6e 64 6c 65 00  00 00 00 00 00 00 00 01  |#bundle.........|
 00000010  00 00 2d 38 2f 53 74 61  74 75 73 2f 44 65 63 6b  |..-8/Status/Deck|
 00000020  2f 53 6f 6e 67 2f 4f 76  65 72 76 69 65 77 00 00  |/Song/Overview..|
 00000030  2c 69 69 69 69 62 00 00  00 00 00 00 00 00 00 f0  |,iiiib..........|
 00000040  00 00 00 10 00 00 00 03  00 00 2d 00 00 00 00 00  |..........-.....|
 00000050  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
 00000060  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
 00000070  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
... truncated
 00002d20  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
 00002d30  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
 00002d40  00 00 00 00 00 00 00 00  00 00 00 00              |............|
}

The number of bytes in the token output you see above that appear before the blob is 76 bytes. The length of the token is 11596 (including those 76 bytes).

11596 - 76 = 11520

So, we can see that 11520 blob bytes are indeed present.

I think the problem might be that reader.Buffered() is not returning what you expect, ie. the full number of bytes in the buffered reader but rather just what you get from a single .Read() on the buffer. There might be a smaller buffer than 11520 bytes in use, and perhaps multiple reads are required?

Just a theory. I could be totally wrong... I can drop an actual payload from a packet capture somewhere if it helps.

mhite commented 1 year ago

I think the default buffer is 4096.

https://cs.opensource.google/go/go/+/master:src/bufio/bufio.go;l=19

Math adds up. I added some more info to the error message so we could see what .Buffered() equals.

err = readBlob: invalid blob length 11520, Buffered() == 4020

4020 + 76 bytes (see previous comment about the 76 byte present before the blob) = 4096.