diff --git a/osc/osc.go b/osc/osc.go index e829ec4..d4b89ca 100644 --- a/osc/osc.go +++ b/osc/osc.go @@ -707,7 +707,7 @@ func readArguments(msg *Message, reader *bufio.Reader, start *int) error { // If the typetag doesn't start with ',', it's not valid if typetags[0] != ',' { - return errors.New("unsupported type tag string") + return fmt.Errorf("unsupported type tag string %s", typetags) } // Remove ',' from the type tag @@ -954,12 +954,8 @@ func readPaddedString(reader *bufio.Reader) (string, int, error) { } n := len(str) - // Remove the string delimiter, in order to calculate the right amount - // of padding bytes - str = str[:len(str)-1] - - // Remove the padding bytes - padLen := padBytesNeeded(len(str)) - 1 + // Remove the padding bytes (leaving the null delimiter) + padLen := padBytesNeeded(len(str)) if padLen > 0 { n += padLen padBytes := make([]byte, padLen) @@ -968,20 +964,30 @@ func readPaddedString(reader *bufio.Reader) (string, int, error) { } } - return str, n, nil + // Strip off the string delimiter + return str[:len(str)-1], n, nil } // writePaddedString writes a string with padding bytes to the a buffer. // Returns, the number of written bytes and an error if any. func writePaddedString(str string, buf *bytes.Buffer) (int, error) { + // Truncate at the first null, just in case there is more than one present + nullIndex := strings.Index(str, "\x00") + if nullIndex > 0 { + str = str[:nullIndex] + } // Write the string to the buffer n, err := buf.WriteString(str) if err != nil { return 0, err } + // Always write a null terminator, as we stripped it earlier if it existed + buf.WriteByte(0) + n += 1 + // Calculate the padding bytes needed and create a buffer for the padding bytes - numPadBytes := padBytesNeeded(len(str)) + numPadBytes := padBytesNeeded(n) if numPadBytes > 0 { padBytes := make([]byte, numPadBytes) // Add the padding bytes to the buffer @@ -998,7 +1004,7 @@ func writePaddedString(str string, buf *bytes.Buffer) (int, error) { // padBytesNeeded determines how many bytes are needed to fill up to the next 4 // byte length. func padBytesNeeded(elementLen int) int { - return 4*(elementLen/4+1) - elementLen + return ((4 - (elementLen % 4)) % 4) } //// diff --git a/osc/osc_test.go b/osc/osc_test.go index 9e5e358..b22fd69 100644 --- a/osc/osc_test.go +++ b/osc/osc_test.go @@ -3,6 +3,7 @@ package osc import ( "bufio" "bytes" + "io" "net" "reflect" "sync" @@ -192,7 +193,7 @@ func TestServerMessageReceiving(t *testing.T) { packet, err := server.ReceivePacket(c) if err != nil { - t.Error("Server error") + t.Errorf("server error: %v", err) return } if packet == nil { @@ -314,45 +315,69 @@ func TestReadPaddedString(t *testing.T) { buf []byte // buffer n int // bytes needed s string // resulting string + e error // expected error }{ - {[]byte{'t', 'e', 's', 't', 's', 't', 'r', 'i', 'n', 'g', 0, 0}, 12, "teststring"}, - {[]byte{'t', 'e', 's', 't', 0, 0, 0, 0}, 8, "test"}, + {[]byte{'t', 'e', 's', 't', 'S', 't', 'r', 'i', 'n', 'g', 0, 0}, 12, "testString", nil}, + {[]byte{'t', 'e', 's', 't', 'e', 'r', 's', 0}, 8, "testers", nil}, + {[]byte{'t', 'e', 's', 't', 's', 0, 0, 0}, 8, "tests", nil}, + {[]byte{'t', 'e', 's', 't', 0, 0, 0, 0}, 8, "test", nil}, + {[]byte{}, 0, "", io.EOF}, + {[]byte{'t', 'e', 's', 0}, 4, "tes", nil}, // OSC uses null terminated strings + {[]byte{'t', 'e', 's', 0, 0, 0, 0, 0}, 4, "tes", nil}, // Additional nulls should be ignored + {[]byte{'t', 'e', 's', 0, 0, 0}, 4, "tes", nil}, // Whether or not the nulls fall on a 4 byte padding boundary + {[]byte{'t', 'e', 's', 't'}, 0, "", io.EOF}, // if there is no null byte at the end, it doesn't work. } { buf := bytes.NewBuffer(tt.buf) s, n, err := readPaddedString(bufio.NewReader(buf)) - if err != nil { - t.Errorf("%s: Error reading padded string: %s", s, err) + if got, want := err, tt.e; got != want { + t.Errorf("%q: Unexpected error reading padded string; got = %q, want = %q", tt.s, got, want) } if got, want := n, tt.n; got != want { - t.Errorf("%s: Bytes needed don't match; got = %d, want = %d", tt.s, got, want) + t.Errorf("%q: Bytes needed don't match; got = %d, want = %d", tt.s, got, want) } if got, want := s, tt.s; got != want { - t.Errorf("%s: Strings don't match; got = %s, want = %s", tt.s, got, want) + t.Errorf("%q: Strings don't match; got = %q, want = %q", tt.s, got, want) } } } func TestWritePaddedString(t *testing.T) { - buf := []byte{} - bytesBuffer := bytes.NewBuffer(buf) - testString := "testString" - expectedNumberOfWrittenBytes := len(testString) + padBytesNeeded(len(testString)) - - n, err := writePaddedString(testString, bytesBuffer) - if err != nil { - t.Errorf(err.Error()) - } + for _, tt := range []struct { + s string // string + buf []byte // resulting buffer + n int // bytes expected + }{ + {"testString", []byte{'t', 'e', 's', 't', 'S', 't', 'r', 'i', 'n', 'g', 0, 0}, 12}, + {"testers", []byte{'t', 'e', 's', 't', 'e', 'r', 's', 0}, 8}, + {"tests", []byte{'t', 'e', 's', 't', 's', 0, 0, 0}, 8}, + {"test", []byte{'t', 'e', 's', 't', 0, 0, 0, 0}, 8}, + {"tes", []byte{'t', 'e', 's', 0}, 4}, + {"tes\x00", []byte{'t', 'e', 's', 0}, 4}, // Don't add a second null terminator if one is already present + {"tes\x00\x00\x00\x00\x00", []byte{'t', 'e', 's', 0}, 4}, // Skip extra nulls + {"tes\x00\x00\x00", []byte{'t', 'e', 's', 0}, 4}, // Even if they don't fall on a 4 byte padding boundary + {"", []byte{0, 0, 0, 0}, 4}, // OSC uses null terminated strings, padded to the 4 byte boundary + } { + buf := []byte{} + bytesBuffer := bytes.NewBuffer(buf) - if n != expectedNumberOfWrittenBytes { - t.Errorf("Expected number of written bytes should be \"%d\" and is \"%d\"", expectedNumberOfWrittenBytes, n) + n, err := writePaddedString(tt.s, bytesBuffer) + if err != nil { + t.Errorf(err.Error()) + } + if got, want := n, tt.n; got != want { + t.Errorf("%q: Count of bytes written don't match; got = %d, want = %d", tt.s, got, want) + } + if got, want := bytesBuffer, tt.buf; !bytes.Equal(got.Bytes(), want) { + t.Errorf("%q: Buffers don't match; got = %q, want = %q", tt.s, got.Bytes(), want) + } } } func TestPadBytesNeeded(t *testing.T) { var n int n = padBytesNeeded(4) - if n != 4 { - t.Errorf("Number of pad bytes should be 4 and is: %d", n) + if n != 0 { + t.Errorf("Number of pad bytes should be 0 and is: %d", n) } n = padBytesNeeded(3) @@ -360,19 +385,34 @@ func TestPadBytesNeeded(t *testing.T) { t.Errorf("Number of pad bytes should be 1 and is: %d", n) } + n = padBytesNeeded(2) + if n != 2 { + t.Errorf("Number of pad bytes should be 2 and is: %d", n) + } + n = padBytesNeeded(1) if n != 3 { t.Errorf("Number of pad bytes should be 3 and is: %d", n) } n = padBytesNeeded(0) - if n != 4 { - t.Errorf("Number of pad bytes should be 4 and is: %d", n) + if n != 0 { + t.Errorf("Number of pad bytes should be 0 and is: %d", n) + } + + n = padBytesNeeded(5) + if n != 3 { + t.Errorf("Number of pad bytes should be 3 and is: %d", n) + } + + n = padBytesNeeded(7) + if n != 1 { + t.Errorf("Number of pad bytes should be 1 and is: %d", n) } n = padBytesNeeded(32) - if n != 4 { - t.Errorf("Number of pad bytes should be 4 and is: %d", n) + if n != 0 { + t.Errorf("Number of pad bytes should be 0 and is: %d", n) } n = padBytesNeeded(63)