Merge pull request #159 from uber/reserved

Ensure reserved bytes are always written as 0

frame.go

@@ -164,6 +164,7 @@ func (f *Frame) write(msg message) error {
 	}
 
 	f.Header.ID = msg.ID()
+	f.Header.reserved1 = 0
 	f.Header.messageType = msg.messageType()
 	f.Header.SetPayloadSize(uint16(wbuf.BytesWritten()))
 	return nil

frame_pool_test.go

@@ -25,6 +25,7 @@ package tchannel_test
 
 import (
 	"bytes"
+	"io"
 	"math/rand"
 	"sync"
 	"testing"
@@ -37,6 +38,7 @@ import (
 	"github.com/uber/tchannel-go/raw"
 	"github.com/uber/tchannel-go/testutils"
 	"github.com/uber/tchannel-go/testutils/goroutines"
+	"github.com/uber/tchannel-go/testutils/testreader"
 	"golang.org/x/net/context"
 )
 
@@ -156,9 +158,8 @@ type dirtyFramePool struct{}
 
 func (p dirtyFramePool) Get() *Frame {
 	f := NewFrame(MaxFramePayloadSize)
-	for i := range f.Payload {
-		f.Payload[i] = ^byte(0)
-	}
+	reader := testreader.Looper([]byte{^byte(0)})
+	io.ReadFull(reader, f.Payload)
 	return f
 }
 

frame_test.go

@@ -28,6 +28,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"github.com/uber/tchannel-go/testutils/testreader"
 	"github.com/uber/tchannel-go/typed"
 )
 
@@ -93,3 +94,27 @@ func TestEmptyPayload(t *testing.T) {
 	// This is also simulated by the LimitedReader so we use that here.
 	require.NoError(t, f.ReadIn(&io.LimitedReader{R: buf, N: FrameHeaderSize}))
 }
+
+func TestReservedBytes(t *testing.T) {
+	// Set up a frame with non-zero values
+	f := NewFrame(MaxFramePayloadSize)
+	reader := testreader.Looper([]byte{^byte(0)})
+	io.ReadFull(reader, f.Payload)
+	f.Header.read(typed.NewReadBuffer(f.Payload))
+
+	m := &pingRes{id: 1}
+	f.write(m)
+
+	buf := &bytes.Buffer{}
+	f.WriteOut(buf)
+	assert.Equal(t,
+		[]byte{
+			0x0, 0x10, // size
+			0xd1,               // type
+			0x0,                // reserved should always be 0
+			0x0, 0x0, 0x0, 0x1, // id
+			0x0, 0x0, 0x0, 0x0, // reserved should always be 0
+			0x0, 0x0, 0x0, 0x0, // reserved should always be 0
+		},
+		buf.Bytes(), "Unexpected bytes")
+}

testutils/testreader/loop.go

@@ -0,0 +1,43 @@
+// Copyright (c) 2015 Uber Technologies, Inc.
+
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package testreader
+
+import "io"
+
+type loopReader struct {
+	bs  []byte
+	pos int
+}
+
+func (r loopReader) Read(p []byte) (int, error) {
+	for i := range p {
+		p[i] = r.bs[r.pos]
+		if r.pos++; r.pos == len(r.bs) {
+			r.pos = 0
+		}
+	}
+	return len(p), nil
+}
+
+// Looper returns a reader that will return the bytes in bs as if it was a circular buffer.
+func Looper(bs []byte) io.Reader {
+	return &loopReader{bs, 0}
+}

testutils/testreader/loop_test.go

@@ -0,0 +1,47 @@
+// Copyright (c) 2015 Uber Technologies, Inc.
+
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package testreader
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestLooper(t *testing.T) {
+	tests := []struct {
+		bs       []byte
+		expected []byte
+	}{
+		{[]byte{0x1}, []byte{0x1, 0x1, 0x1}},
+		{[]byte{0x1, 0x2}, []byte{0x1, 0x2, 0x1}},
+		{[]byte{0x1, 0x2}, []byte{0x1, 0x2, 0x1, 0x2, 0x1, 0x2}},
+	}
+
+	for _, tt := range tests {
+		r := Looper(tt.bs)
+		got := make([]byte, len(tt.expected))
+		n, err := r.Read(got)
+		assert.NoError(t, err, "Read failed")
+		assert.Equal(t, len(got), n)
+		assert.Equal(t, tt.expected, got, "Got unexpected bytes")
+	}
+}