encoding: refactor to remove common RequestID from packets

encoding/ssh/filexfer/extended_packets.go

@@ -12,7 +12,6 @@ type ExtendedData = interface {
 
 // ExtendedPacket defines the SSH_FXP_CLOSE packet.
 type ExtendedPacket struct {
-	RequestID       uint32
 	ExtendedRequest string
 
 	Data ExtendedData
@@ -21,10 +20,10 @@ type ExtendedPacket struct {
 // MarshalPacket returns p as a two-part binary encoding of p.
 //
 // The Data is marshaled into binary, and returned as the payload.
-func (p *ExtendedPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *ExtendedPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 + len(p.ExtendedRequest) // string(extended-request)
 
-	b := NewMarshalBuffer(PacketTypeExtended, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeExtended, reqid, size)
 
 	b.AppendString(p.ExtendedRequest)
 
@@ -38,11 +37,6 @@ func (p *ExtendedPacket) MarshalPacket() (header, payload []byte, err error) {
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *ExtendedPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 //
@@ -66,31 +60,16 @@ func (p *ExtendedPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *ExtendedPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // ExtendedReplyPacket defines the SSH_FXP_CLOSE packet.
 type ExtendedReplyPacket struct {
-	RequestID uint32
-
 	Data ExtendedData
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
 //
 // The Data is marshaled into binary, and returned as the payload.
-func (p *ExtendedReplyPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *ExtendedReplyPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
-	b := NewMarshalBuffer(PacketTypeExtendedReply, p.RequestID, 0)
+	b := NewMarshalBuffer(PacketTypeExtendedReply, reqid, 0)
 
 	if p.Data != nil {
 		payload, err = p.Data.MarshalBinary()
@@ -102,11 +81,6 @@ func (p *ExtendedReplyPacket) MarshalPacket() (header, payload []byte, err error
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *ExtendedReplyPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 //
@@ -125,16 +99,3 @@ func (p *ExtendedReplyPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 
 	return nil
 }
-
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *ExtendedReplyPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}

encoding/ssh/filexfer/extended_packets_test.go

@@ -30,6 +30,8 @@ func (d *testExtendedData) UnmarshalBinary(data []byte) error {
 	return nil
 }
 
+var _ Packet = &ExtendedPacket{}
+
 func TestExtendedPacketNoData(t *testing.T) {
 	const (
 		id              = 42
@@ -37,11 +39,10 @@ func TestExtendedPacketNoData(t *testing.T) {
 	)
 
 	p := &ExtendedPacket{
-		RequestID:       id,
 		ExtendedRequest: extendedRequest,
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -59,17 +60,13 @@ func TestExtendedPacketNoData(t *testing.T) {
 
 	*p = ExtendedPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.ExtendedRequest != extendedRequest {
-		t.Errorf("UnmarshalBinary(): ExtendedRequest was %q, but expected %q", p.ExtendedRequest, extendedRequest)
+		t.Errorf("UnmarshalPacketBody(): ExtendedRequest was %q, but expected %q", p.ExtendedRequest, extendedRequest)
 	}
 }
 
@@ -83,14 +80,13 @@ func TestExtendedPacketTestData(t *testing.T) {
 	const value = 13
 
 	p := &ExtendedPacket{
-		RequestID:       id,
 		ExtendedRequest: extendedRequest,
 		Data: &testExtendedData{
 			value: textValue,
 		},
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -111,61 +107,53 @@ func TestExtendedPacketTestData(t *testing.T) {
 		Data: new(testExtendedData),
 	}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.ExtendedRequest != extendedRequest {
-		t.Errorf("UnmarshalBinary(): ExtendedRequest was %q, but expected %q", p.ExtendedRequest, extendedRequest)
+		t.Errorf("UnmarshalPacketBody(): ExtendedRequest was %q, but expected %q", p.ExtendedRequest, extendedRequest)
 	}
 
 	if data, ok := p.Data.(*testExtendedData); !ok {
-		t.Errorf("UnmarshalBinary(): Data was type %T, but expected %T", p.Data, data)
+		t.Errorf("UnmarshalPacketBody(): Data was type %T, but expected %T", p.Data, data)
 
 	} else if data.value != value {
-		t.Errorf("UnmarshalBinary(): Data.value was %#x, but expected %#x", data.value, value)
+		t.Errorf("UnmarshalPacketBody(): Data.value was %#x, but expected %#x", data.value, value)
 	}
 
 	*p = ExtendedPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.ExtendedRequest != extendedRequest {
-		t.Errorf("UnmarshalBinary(): ExtendedRequest was %q, but expected %q", p.ExtendedRequest, extendedRequest)
+		t.Errorf("UnmarshalPacketBody(): ExtendedRequest was %q, but expected %q", p.ExtendedRequest, extendedRequest)
 	}
 
 	wantBuffer := []byte{0x27}
 
 	if data, ok := p.Data.(*Buffer); !ok {
-		t.Errorf("UnmarshalBinary(): Data was type %T, but expected %T", p.Data, data)
+		t.Errorf("UnmarshalPacketBody(): Data was type %T, but expected %T", p.Data, data)
 
 	} else if !bytes.Equal(data.b, wantBuffer) {
-		t.Errorf("UnmarshalBinary(): Data was %X, but expected %X", data.b, wantBuffer)
+		t.Errorf("UnmarshalPacketBody(): Data was %X, but expected %X", data.b, wantBuffer)
 	}
 }
 
+var _ Packet = &ExtendedReplyPacket{}
+
 func TestExtendedReplyNoData(t *testing.T) {
 	const (
 		id = 42
 	)
 
-	p := &ExtendedReplyPacket{
+	p := &ExtendedReplyPacket{}
-		RequestID: id,
-	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -182,14 +170,10 @@ func TestExtendedReplyNoData(t *testing.T) {
 
 	*p = ExtendedReplyPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
-
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
 }
 
 func TestExtendedReplyPacketTestData(t *testing.T) {
@@ -201,13 +185,12 @@ func TestExtendedReplyPacketTestData(t *testing.T) {
 	const value = 13
 
 	p := &ExtendedReplyPacket{
-		RequestID: id,
 		Data: &testExtendedData{
 			value: textValue,
 		},
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -227,39 +210,31 @@ func TestExtendedReplyPacketTestData(t *testing.T) {
 		Data: new(testExtendedData),
 	}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if data, ok := p.Data.(*testExtendedData); !ok {
-		t.Errorf("UnmarshalBinary(): Data was type %T, but expected %T", p.Data, data)
+		t.Errorf("UnmarshalPacketBody(): Data was type %T, but expected %T", p.Data, data)
 
 	} else if data.value != value {
-		t.Errorf("UnmarshalBinary(): Data.value was %#x, but expected %#x", data.value, value)
+		t.Errorf("UnmarshalPacketBody(): Data.value was %#x, but expected %#x", data.value, value)
 	}
 
 	*p = ExtendedReplyPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	wantBuffer := []byte{0x27}
 
 	if data, ok := p.Data.(*Buffer); !ok {
-		t.Errorf("UnmarshalBinary(): Data was type %T, but expected %T", p.Data, data)
+		t.Errorf("UnmarshalPacketBody(): Data was type %T, but expected %T", p.Data, data)
 
 	} else if !bytes.Equal(data.b, wantBuffer) {
-		t.Errorf("UnmarshalBinary(): Data was %X, but expected %X", data.b, wantBuffer)
+		t.Errorf("UnmarshalPacketBody(): Data was %X, but expected %X", data.b, wantBuffer)
 	}
 }

encoding/ssh/filexfer/filexfer.go

@@ -2,11 +2,11 @@ package filexfer
 
 // Packet defines the behavior of an SFTP packet.
 type Packet interface {
-	MarshalPacket() (header, payload []byte, err error)
+	MarshalPacket(reqid uint32) (header, payload []byte, err error)
 	UnmarshalPacketBody(buf *Buffer) error
 }
 
-// ComposePacket converts returns from MarshalPacket into the returns expected by MarshalBinary.
+// ComposePacket converts returns from MarshalPacket into an equivalent call to MarshalBinary.
 func ComposePacket(header, payload []byte, err error) ([]byte, error) {
 	return append(header, payload...), err
 }

encoding/ssh/filexfer/handle_packets.go

@@ -2,26 +2,20 @@ package filexfer
 
 // ClosePacket defines the SSH_FXP_CLOSE packet.
 type ClosePacket struct {
-	RequestID uint32
+	Handle string
-	Handle    string
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *ClosePacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *ClosePacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 + len(p.Handle) // string(handle)
 
-	b := NewMarshalBuffer(PacketTypeClose, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeClose, reqid, size)
 
 	b.AppendString(p.Handle)
 
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *ClosePacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *ClosePacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -32,33 +26,19 @@ func (p *ClosePacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *ClosePacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // ReadPacket defines the SSH_FXP_READ packet.
 type ReadPacket struct {
-	RequestID uint32
+	Handle string
-	Handle    string
+	Offset uint64
-	Offset    uint64
+	Len    uint32
-	Len       uint32
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *ReadPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *ReadPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	// string(handle) + uint64(offset) + uint32(len)
 	size := 4 + len(p.Handle) + 8 + 4
 
-	b := NewMarshalBuffer(PacketTypeRead, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeRead, reqid, size)
 
 	b.AppendString(p.Handle)
 	b.AppendUint64(p.Offset)
@@ -67,11 +47,6 @@ func (p *ReadPacket) MarshalPacket() (header, payload []byte, err error) {
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *ReadPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *ReadPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -90,33 +65,19 @@ func (p *ReadPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *ReadPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // WritePacket defines the SSH_FXP_WRITE packet.
 type WritePacket struct {
-	RequestID uint32
+	Handle string
-	Handle    string
+	Offset uint64
-	Offset    uint64
+	Data   []byte
-	Data      []byte
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *WritePacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *WritePacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	// string(handle) + uint64(offset) + uint32(len(data)); data content in payload
 	size := 4 + len(p.Handle) + 8 + 4
 
-	b := NewMarshalBuffer(PacketTypeWrite, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeWrite, reqid, size)
 
 	b.AppendString(p.Handle)
 	b.AppendUint64(p.Offset)
@@ -125,11 +86,6 @@ func (p *WritePacket) MarshalPacket() (header, payload []byte, err error) {
 	return b.Packet(p.Data)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *WritePacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *WritePacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -148,41 +104,22 @@ func (p *WritePacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *WritePacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // FstatPacket defines the SSH_FXP_FSTAT packet.
 type FstatPacket struct {
-	RequestID uint32
+	Handle string
-	Handle    string
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *FstatPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *FstatPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 + len(p.Handle) // string(handle)
 
-	b := NewMarshalBuffer(PacketTypeFstat, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeFstat, reqid, size)
 
 	b.AppendString(p.Handle)
 
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *FstatPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *FstatPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -193,31 +130,17 @@ func (p *FstatPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *FstatPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // FsetstatPacket defines the SSH_FXP_FSETSTAT packet.
 type FsetstatPacket struct {
-	RequestID uint32
+	Handle string
-	Handle    string
+	Attrs  Attributes
-	Attrs     Attributes
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *FsetstatPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *FsetstatPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 + len(p.Handle) + p.Attrs.Len() // string(handle) + ATTRS(attrs)
 
-	b := NewMarshalBuffer(PacketTypeFsetstat, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeFsetstat, reqid, size)
 
 	b.AppendString(p.Handle)
 
@@ -226,11 +149,6 @@ func (p *FsetstatPacket) MarshalPacket() (header, payload []byte, err error) {
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *FsetstatPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *FsetstatPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -241,41 +159,22 @@ func (p *FsetstatPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return p.Attrs.UnmarshalFrom(buf)
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *FsetstatPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // ReaddirPacket defines the SSH_FXP_READDIR packet.
 type ReaddirPacket struct {
-	RequestID uint32
+	Handle string
-	Handle    string
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *ReaddirPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *ReaddirPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 + len(p.Handle) // string(handle)
 
-	b := NewMarshalBuffer(PacketTypeReaddir, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeReaddir, reqid, size)
 
 	b.AppendString(p.Handle)
 
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *ReaddirPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *ReaddirPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -285,16 +184,3 @@ func (p *ReaddirPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 
 	return nil
 }
-
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *ReaddirPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}

encoding/ssh/filexfer/handle_packets_test.go

@@ -5,6 +5,8 @@ import (
 	"testing"
 )
 
+var _ Packet = &ClosePacket{}
+
 func TestClosePacket(t *testing.T) {
 	const (
 		id     = 42
@@ -12,11 +14,10 @@ func TestClosePacket(t *testing.T) {
 	)
 
 	p := &ClosePacket{
-		RequestID: id,
+		Handle: "somehandle",
-		Handle:    "somehandle",
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -34,20 +35,18 @@ func TestClosePacket(t *testing.T) {
 
 	*p = ClosePacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Handle != handle {
-		t.Errorf("UnmarshalBinary(): Handle was %q, but expected %q", p.Handle, handle)
+		t.Errorf("UnmarshalPacketBody(): Handle was %q, but expected %q", p.Handle, handle)
 	}
 }
 
+var _ Packet = &ReadPacket{}
+
 func TestReadPacket(t *testing.T) {
 	const (
 		id     = 42
@@ -57,13 +56,12 @@ func TestReadPacket(t *testing.T) {
 	)
 
 	p := &ReadPacket{
-		RequestID: id,
+		Handle: "somehandle",
-		Handle:    "somehandle",
+		Offset: offset,
-		Offset:    offset,
+		Len:    length,
-		Len:       length,
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -83,28 +81,26 @@ func TestReadPacket(t *testing.T) {
 
 	*p = ReadPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Handle != handle {
-		t.Errorf("UnmarshalBinary(): Handle was %q, but expected %q", p.Handle, handle)
+		t.Errorf("UnmarshalPacketBody(): Handle was %q, but expected %q", p.Handle, handle)
 	}
 
 	if p.Offset != offset {
-		t.Errorf("UnmarshalBinary(): Offset was %x, but expected %x", p.Offset, offset)
+		t.Errorf("UnmarshalPacketBody(): Offset was %x, but expected %x", p.Offset, offset)
 	}
 
 	if p.Len != length {
-		t.Errorf("UnmarshalBinary(): Len was %x, but expected %x", p.Len, length)
+		t.Errorf("UnmarshalPacketBody(): Len was %x, but expected %x", p.Len, length)
 	}
 }
 
+var _ Packet = &WritePacket{}
+
 func TestWritePacket(t *testing.T) {
 	const (
 		id     = 42
@@ -115,13 +111,12 @@ func TestWritePacket(t *testing.T) {
 	var payload = []byte(`foobar`)
 
 	p := &WritePacket{
-		RequestID: id,
+		Handle: "somehandle",
-		Handle:    "somehandle",
+		Offset: offset,
-		Offset:    offset,
+		Data:   payload,
-		Data:      payload,
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -141,28 +136,26 @@ func TestWritePacket(t *testing.T) {
 
 	*p = WritePacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Handle != handle {
-		t.Errorf("UnmarshalBinary(): Handle was %q, but expected %q", p.Handle, handle)
+		t.Errorf("UnmarshalPacketBody(): Handle was %q, but expected %q", p.Handle, handle)
 	}
 
 	if p.Offset != offset {
-		t.Errorf("UnmarshalBinary(): Offset was %x, but expected %x", p.Offset, offset)
+		t.Errorf("UnmarshalPacketBody(): Offset was %x, but expected %x", p.Offset, offset)
 	}
 
 	if !bytes.Equal(p.Data, payload) {
-		t.Errorf("UnmarshalBinary(): Data was %X, but expected %X", p.Data, payload)
+		t.Errorf("UnmarshalPacketBody(): Data was %X, but expected %X", p.Data, payload)
 	}
 }
 
+var _ Packet = &FstatPacket{}
+
 func TestFstatPacket(t *testing.T) {
 	const (
 		id     = 42
@@ -170,11 +163,10 @@ func TestFstatPacket(t *testing.T) {
 	)
 
 	p := &FstatPacket{
-		RequestID: id,
+		Handle: "somehandle",
-		Handle:    "somehandle",
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -192,20 +184,18 @@ func TestFstatPacket(t *testing.T) {
 
 	*p = FstatPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Handle != handle {
-		t.Errorf("UnmarshalBinary(): Handle was %q, but expected %q", p.Handle, handle)
+		t.Errorf("UnmarshalPacketBody(): Handle was %q, but expected %q", p.Handle, handle)
 	}
 }
 
+var _ Packet = &FsetstatPacket{}
+
 func TestFsetstatPacket(t *testing.T) {
 	const (
 		id     = 42
@@ -214,15 +204,14 @@ func TestFsetstatPacket(t *testing.T) {
 	)
 
 	p := &FsetstatPacket{
-		RequestID: id,
+		Handle: "somehandle",
-		Handle:    "somehandle",
 		Attrs: Attributes{
 			Flags:       AttrPermissions,
 			Permissions: perms,
 		},
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -242,20 +231,18 @@ func TestFsetstatPacket(t *testing.T) {
 
 	*p = FsetstatPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Handle != handle {
-		t.Errorf("UnmarshalBinary(): Handle was %q, but expected %q", p.Handle, handle)
+		t.Errorf("UnmarshalPacketBody(): Handle was %q, but expected %q", p.Handle, handle)
 	}
 }
 
+var _ Packet = &ReaddirPacket{}
+
 func TestReaddirPacket(t *testing.T) {
 	const (
 		id     = 42
@@ -263,11 +250,10 @@ func TestReaddirPacket(t *testing.T) {
 	)
 
 	p := &ReaddirPacket{
-		RequestID: id,
+		Handle: "somehandle",
-		Handle:    "somehandle",
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -285,16 +271,12 @@ func TestReaddirPacket(t *testing.T) {
 
 	*p = ReaddirPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Handle != handle {
-		t.Errorf("UnmarshalBinary(): Handle was %q, but expected %q", p.Handle, handle)
+		t.Errorf("UnmarshalPacketBody(): Handle was %q, but expected %q", p.Handle, handle)
 	}
 }

encoding/ssh/filexfer/init_packets.go

@@ -6,8 +6,8 @@ type InitPacket struct {
 	Extensions []*ExtensionPair
 }
 
-// MarshalPacket returns p as a two-part binary encoding of p.
+// MarshalBinary returns p as the binary encoding of p.
-func (p *InitPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *InitPacket) MarshalBinary() ([]byte, error) {
 	size := 1 + 4 // byte(type) + uint32(version)
 
 	for _, ext := range p.Extensions {
@@ -24,16 +24,15 @@ func (p *InitPacket) MarshalPacket() (header, payload []byte, err error) {
 
 	b.PutLength(size)
 
-	return b.Bytes(), nil, nil
+	return b.Bytes(), nil
 }
 
-// MarshalBinary returns p as the binary encoding of p.
+// UnmarshalBinary unmarshals a full raw packet out of the given data.
-func (p *InitPacket) MarshalBinary() ([]byte, error) {
+// It is assumed that the uint32(length) has already been consumed to receive the data.
-	return ComposePacket(p.MarshalPacket())
+// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-}
+func (p *InitPacket) UnmarshalBinary(data []byte) (err error) {
+	buf := NewBuffer(data)
 
-// UnmarshalPacketBody unmarshals the packet body from the given Buffer.
-func (p *InitPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	if p.Version, err = buf.ConsumeUint32(); err != nil {
 		return err
 	}
@@ -50,21 +49,14 @@ func (p *InitPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *InitPacket) UnmarshalBinary(data []byte) error {
-	return p.UnmarshalPacketBody(NewBuffer(data))
-}
-
 // VersionPacket defines the SSH_FXP_VERSION packet.
 type VersionPacket struct {
 	Version    uint32
 	Extensions []*ExtensionPair
 }
 
-// MarshalPacket returns p as a two-part binary encoding of p.
+// MarshalBinary returns p as the binary encoding of p.
-func (p *VersionPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *VersionPacket) MarshalBinary() ([]byte, error) {
 	size := 1 + 4 // byte(type) + uint32(version)
 
 	for _, ext := range p.Extensions {
@@ -81,16 +73,15 @@ func (p *VersionPacket) MarshalPacket() (header, payload []byte, err error) {
 
 	b.PutLength(size)
 
-	return b.Bytes(), nil, nil
+	return b.Bytes(), nil
 }
 
-// MarshalBinary returns p as the binary encoding of p.
+// UnmarshalBinary unmarshals a full raw packet out of the given data.
-func (p *VersionPacket) MarshalBinary() ([]byte, error) {
+// It is assumed that the uint32(length) has already been consumed to receive the data.
-	return ComposePacket(p.MarshalPacket())
+// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-}
+func (p *VersionPacket) UnmarshalBinary(data []byte) (err error) {
+	buf := NewBuffer(data)
 
-// UnmarshalPacketBody unmarshals the packet body from the given Buffer.
-func (p *VersionPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	if p.Version, err = buf.ConsumeUint32(); err != nil {
 		return err
 	}
@@ -106,10 +97,3 @@ func (p *VersionPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 
 	return nil
 }
-
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *VersionPacket) UnmarshalBinary(data []byte) error {
-	return p.UnmarshalPacketBody(NewBuffer(data))
-}

encoding/ssh/filexfer/open_packets.go

@@ -12,18 +12,17 @@ const (
 
 // OpenPacket defines the SSH_FXP_OPEN packet.
 type OpenPacket struct {
-	RequestID uint32
+	Filename string
-	Filename  string
+	PFlags   uint32
-	PFlags    uint32
+	Attrs    Attributes
-	Attrs     Attributes
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *OpenPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *OpenPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	// string(filename) + uint32(pflags) + ATTRS(attrs)
 	size := 4 + len(p.Filename) + 4 + p.Attrs.Len()
 
-	b := NewMarshalBuffer(PacketTypeOpen, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeOpen, reqid, size)
 
 	b.AppendString(p.Filename)
 	b.AppendUint32(p.PFlags)
@@ -33,11 +32,6 @@ func (p *OpenPacket) MarshalPacket() (header, payload []byte, err error) {
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *OpenPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *OpenPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -52,41 +46,22 @@ func (p *OpenPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return p.Attrs.UnmarshalFrom(buf)
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *OpenPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // OpendirPacket defines the SSH_FXP_OPENDIR packet.
 type OpendirPacket struct {
-	RequestID uint32
+	Path string
-	Path      string
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *OpendirPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *OpendirPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 + len(p.Path) // string(path)
 
-	b := NewMarshalBuffer(PacketTypeOpendir, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeOpendir, reqid, size)
 
 	b.AppendString(p.Path)
 
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *OpendirPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *OpendirPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -96,16 +71,3 @@ func (p *OpendirPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 
 	return nil
 }
-
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *OpendirPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}

encoding/ssh/filexfer/open_packets_test.go

@@ -5,6 +5,8 @@ import (
 	"testing"
 )
 
+var _ Packet = &OpenPacket{}
+
 func TestOpenPacket(t *testing.T) {
 	const (
 		id       = 42
@@ -13,16 +15,15 @@ func TestOpenPacket(t *testing.T) {
 	)
 
 	p := &OpenPacket{
-		RequestID: id,
+		Filename: "/foo",
-		Filename:  "/foo",
+		PFlags:   FlagRead,
-		PFlags:    FlagRead,
 		Attrs: Attributes{
 			Flags:       AttrPermissions,
 			Permissions: perms,
 		},
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -43,32 +44,30 @@ func TestOpenPacket(t *testing.T) {
 
 	*p = OpenPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Filename != filename {
-		t.Errorf("UnmarshalBinary(): Filename was %q, but expected %q", p.Filename, filename)
+		t.Errorf("UnmarshalPacketBody(): Filename was %q, but expected %q", p.Filename, filename)
 	}
 
 	if p.PFlags != FlagRead {
-		t.Errorf("UnmarshalBinary(): PFlags was %#x, but expected %#x", p.PFlags, FlagRead)
+		t.Errorf("UnmarshalPacketBody(): PFlags was %#x, but expected %#x", p.PFlags, FlagRead)
 	}
 
 	if p.Attrs.Flags != AttrPermissions {
-		t.Errorf("UnmarshalBinary(): Attrs.Flags was %#x, but expected %#x", p.Attrs.Flags, AttrPermissions)
+		t.Errorf("UnmarshalPacketBody(): Attrs.Flags was %#x, but expected %#x", p.Attrs.Flags, AttrPermissions)
 	}
 
 	if p.Attrs.Permissions != perms {
-		t.Errorf("UnmarshalBinary(): Attrs.Permissions was %#x, but expected %#x", p.Attrs.Permissions, perms)
+		t.Errorf("UnmarshalPacketBody(): Attrs.Permissions was %#x, but expected %#x", p.Attrs.Permissions, perms)
 	}
 }
 
+var _ Packet = &OpendirPacket{}
+
 func TestOpendirPacket(t *testing.T) {
 	const (
 		id   = 42
@@ -76,11 +75,10 @@ func TestOpendirPacket(t *testing.T) {
 	)
 
 	p := &OpendirPacket{
-		RequestID: id,
+		Path: path,
-		Path:      path,
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -98,16 +96,12 @@ func TestOpendirPacket(t *testing.T) {
 
 	*p = OpendirPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Path != path {
-		t.Errorf("UnmarshalBinary(): Path was %q, but expected %q", p.Path, path)
+		t.Errorf("UnmarshalPacketBody(): Path was %q, but expected %q", p.Path, path)
 	}
 }

encoding/ssh/filexfer/path_packets.go

@@ -2,26 +2,20 @@ package filexfer
 
 // LstatPacket defines the SSH_FXP_LSTAT packet.
 type LstatPacket struct {
-	RequestID uint32
+	Path string
-	Path      string
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *LstatPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *LstatPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 + len(p.Path) // string(path)
 
-	b := NewMarshalBuffer(PacketTypeLstat, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeLstat, reqid, size)
 
 	b.AppendString(p.Path)
 
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *LstatPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *LstatPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -32,31 +26,17 @@ func (p *LstatPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *LstatPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // SetstatPacket defines the SSH_FXP_SETSTAT packet.
 type SetstatPacket struct {
-	RequestID uint32
+	Path  string
-	Path      string
+	Attrs Attributes
-	Attrs     Attributes
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *SetstatPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *SetstatPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 + len(p.Path) + p.Attrs.Len() // string(path) + ATTRS(attrs)
 
-	b := NewMarshalBuffer(PacketTypeSetstat, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeSetstat, reqid, size)
 
 	b.AppendString(p.Path)
 
@@ -65,11 +45,6 @@ func (p *SetstatPacket) MarshalPacket() (header, payload []byte, err error) {
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *SetstatPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *SetstatPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -80,41 +55,22 @@ func (p *SetstatPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return p.Attrs.UnmarshalFrom(buf)
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *SetstatPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // RemovePacket defines the SSH_FXP_REMOVE packet.
 type RemovePacket struct {
-	RequestID uint32
+	Path string
-	Path      string
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *RemovePacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *RemovePacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 + len(p.Path) // string(path)
 
-	b := NewMarshalBuffer(PacketTypeRemove, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeRemove, reqid, size)
 
 	b.AppendString(p.Path)
 
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *RemovePacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *RemovePacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -125,31 +81,17 @@ func (p *RemovePacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *RemovePacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // MkdirPacket defines the SSH_FXP_MKDIR packet.
 type MkdirPacket struct {
-	RequestID uint32
+	Path  string
-	Path      string
+	Attrs Attributes
-	Attrs     Attributes
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *MkdirPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *MkdirPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 + len(p.Path) + p.Attrs.Len() // string(path) + ATTRS(attrs)
 
-	b := NewMarshalBuffer(PacketTypeMkdir, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeMkdir, reqid, size)
 
 	b.AppendString(p.Path)
 
@@ -158,11 +100,6 @@ func (p *MkdirPacket) MarshalPacket() (header, payload []byte, err error) {
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *MkdirPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *MkdirPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -173,41 +110,22 @@ func (p *MkdirPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return p.Attrs.UnmarshalFrom(buf)
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *MkdirPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // RmdirPacket defines the SSH_FXP_RMDIR packet.
 type RmdirPacket struct {
-	RequestID uint32
+	Path string
-	Path      string
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *RmdirPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *RmdirPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 + len(p.Path) // string(path)
 
-	b := NewMarshalBuffer(PacketTypeRmdir, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeRmdir, reqid, size)
 
 	b.AppendString(p.Path)
 
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *RmdirPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *RmdirPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -218,41 +136,22 @@ func (p *RmdirPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *RmdirPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // RealpathPacket defines the SSH_FXP_REALPATH packet.
 type RealpathPacket struct {
-	RequestID uint32
+	Path string
-	Path      string
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *RealpathPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *RealpathPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 + len(p.Path) // string(path)
 
-	b := NewMarshalBuffer(PacketTypeRealpath, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeRealpath, reqid, size)
 
 	b.AppendString(p.Path)
 
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *RealpathPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *RealpathPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -263,41 +162,22 @@ func (p *RealpathPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *RealpathPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // StatPacket defines the SSH_FXP_STAT packet.
 type StatPacket struct {
-	RequestID uint32
+	Path string
-	Path      string
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *StatPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *StatPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 + len(p.Path) // string(path)
 
-	b := NewMarshalBuffer(PacketTypeStat, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeStat, reqid, size)
 
 	b.AppendString(p.Path)
 
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *StatPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *StatPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -308,32 +188,18 @@ func (p *StatPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *StatPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // RenamePacket defines the SSH_FXP_RENAME packet.
 type RenamePacket struct {
-	RequestID uint32
+	OldPath string
-	OldPath   string
+	NewPath string
-	NewPath   string
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *RenamePacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *RenamePacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	// string(oldpath) + string(newpath)
 	size := 4 + len(p.OldPath) + 4 + len(p.NewPath)
 
-	b := NewMarshalBuffer(PacketTypeRename, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeRename, reqid, size)
 
 	b.AppendString(p.OldPath)
 	b.AppendString(p.NewPath)
@@ -341,11 +207,6 @@ func (p *RenamePacket) MarshalPacket() (header, payload []byte, err error) {
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *RenamePacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *RenamePacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -360,41 +221,22 @@ func (p *RenamePacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *RenamePacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // ReadlinkPacket defines the SSH_FXP_READLINK packet.
 type ReadlinkPacket struct {
-	RequestID uint32
+	Path string
-	Path      string
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *ReadlinkPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *ReadlinkPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 + len(p.Path) // string(path)
 
-	b := NewMarshalBuffer(PacketTypeReadlink, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeReadlink, reqid, size)
 
 	b.AppendString(p.Path)
 
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *ReadlinkPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *ReadlinkPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -405,36 +247,22 @@ func (p *ReadlinkPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *ReadlinkPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // SymlinkPacket defines the SSH_FXP_SYMLINK packet.
 //
 // The order of the arguments to the SSH_FXP_SYMLINK method was inadvertently reversed.
 // Unfortunately, the reversal was not noticed until the server was widely deployed.
 // Covered in Section 3.1 of https://github.com/openssh/openssh-portable/blob/master/PROTOCOL
 type SymlinkPacket struct {
-	RequestID  uint32
 	LinkPath   string
 	TargetPath string
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *SymlinkPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *SymlinkPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	// string(targetpath) + string(linkpath)
 	size := 4 + len(p.TargetPath) + 4 + len(p.LinkPath)
 
-	b := NewMarshalBuffer(PacketTypeSymlink, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeSymlink, reqid, size)
 
 	// Arguments were inadvertently reversed.
 	b.AppendString(p.TargetPath)
@@ -443,11 +271,6 @@ func (p *SymlinkPacket) MarshalPacket() (header, payload []byte, err error) {
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *SymlinkPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *SymlinkPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -462,16 +285,3 @@ func (p *SymlinkPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 
 	return nil
 }
-
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *SymlinkPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}

encoding/ssh/filexfer/path_packets_test.go

@@ -5,6 +5,8 @@ import (
 	"testing"
 )
 
+var _ Packet = &LstatPacket{}
+
 func TestLstatPacket(t *testing.T) {
 	const (
 		id   = 42
@@ -12,11 +14,10 @@ func TestLstatPacket(t *testing.T) {
 	)
 
 	p := &LstatPacket{
-		RequestID: id,
+		Path: path,
-		Path:      path,
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -34,20 +35,18 @@ func TestLstatPacket(t *testing.T) {
 
 	*p = LstatPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Path != path {
-		t.Errorf("UnmarshalBinary(): Path was %q, but expected %q", p.Path, path)
+		t.Errorf("UnmarshalPacketBody(): Path was %q, but expected %q", p.Path, path)
 	}
 }
 
+var _ Packet = &SetstatPacket{}
+
 func TestSetstatPacket(t *testing.T) {
 	const (
 		id    = 42
@@ -56,15 +55,14 @@ func TestSetstatPacket(t *testing.T) {
 	)
 
 	p := &SetstatPacket{
-		RequestID: id,
+		Path: "/foo",
-		Path:      "/foo",
 		Attrs: Attributes{
 			Flags:       AttrPermissions,
 			Permissions: perms,
 		},
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -84,28 +82,26 @@ func TestSetstatPacket(t *testing.T) {
 
 	*p = SetstatPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Path != path {
-		t.Errorf("UnmarshalBinary(): Path was %q, but expected %q", p.Path, path)
+		t.Errorf("UnmarshalPacketBody(): Path was %q, but expected %q", p.Path, path)
 	}
 
 	if p.Attrs.Flags != AttrPermissions {
-		t.Errorf("UnmarshalBinary(): Attrs.Flags was %#x, but expected %#x", p.Attrs.Flags, AttrPermissions)
+		t.Errorf("UnmarshalPacketBody(): Attrs.Flags was %#x, but expected %#x", p.Attrs.Flags, AttrPermissions)
 	}
 
 	if p.Attrs.Permissions != perms {
-		t.Errorf("UnmarshalBinary(): Attrs.Permissions was %#x, but expected %#x", p.Attrs.Permissions, perms)
+		t.Errorf("UnmarshalPacketBody(): Attrs.Permissions was %#x, but expected %#x", p.Attrs.Permissions, perms)
 	}
 }
 
+var _ Packet = &RemovePacket{}
+
 func TestRemovePacket(t *testing.T) {
 	const (
 		id   = 42
@@ -113,11 +109,10 @@ func TestRemovePacket(t *testing.T) {
 	)
 
 	p := &RemovePacket{
-		RequestID: id,
+		Path: path,
-		Path:      path,
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -135,20 +130,18 @@ func TestRemovePacket(t *testing.T) {
 
 	*p = RemovePacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Path != path {
-		t.Errorf("UnmarshalBinary(): Path was %q, but expected %q", p.Path, path)
+		t.Errorf("UnmarshalPacketBody(): Path was %q, but expected %q", p.Path, path)
 	}
 }
 
+var _ Packet = &MkdirPacket{}
+
 func TestMkdirPacket(t *testing.T) {
 	const (
 		id    = 42
@@ -157,15 +150,14 @@ func TestMkdirPacket(t *testing.T) {
 	)
 
 	p := &MkdirPacket{
-		RequestID: id,
+		Path: "/foo",
-		Path:      "/foo",
 		Attrs: Attributes{
 			Flags:       AttrPermissions,
 			Permissions: perms,
 		},
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -185,28 +177,26 @@ func TestMkdirPacket(t *testing.T) {
 
 	*p = MkdirPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Path != path {
-		t.Errorf("UnmarshalBinary(): Path was %q, but expected %q", p.Path, path)
+		t.Errorf("UnmarshalPacketBody(): Path was %q, but expected %q", p.Path, path)
 	}
 
 	if p.Attrs.Flags != AttrPermissions {
-		t.Errorf("UnmarshalBinary(): Attrs.Flags was %#x, but expected %#x", p.Attrs.Flags, AttrPermissions)
+		t.Errorf("UnmarshalPacketBody(): Attrs.Flags was %#x, but expected %#x", p.Attrs.Flags, AttrPermissions)
 	}
 
 	if p.Attrs.Permissions != perms {
-		t.Errorf("UnmarshalBinary(): Attrs.Permissions was %#x, but expected %#x", p.Attrs.Permissions, perms)
+		t.Errorf("UnmarshalPacketBody(): Attrs.Permissions was %#x, but expected %#x", p.Attrs.Permissions, perms)
 	}
 }
 
+var _ Packet = &RmdirPacket{}
+
 func TestRmdirPacket(t *testing.T) {
 	const (
 		id   = 42
@@ -214,11 +204,10 @@ func TestRmdirPacket(t *testing.T) {
 	)
 
 	p := &RmdirPacket{
-		RequestID: id,
+		Path: path,
-		Path:      path,
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -236,20 +225,18 @@ func TestRmdirPacket(t *testing.T) {
 
 	*p = RmdirPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Path != path {
-		t.Errorf("UnmarshalBinary(): Path was %q, but expected %q", p.Path, path)
+		t.Errorf("UnmarshalPacketBody(): Path was %q, but expected %q", p.Path, path)
 	}
 }
 
+var _ Packet = &RealpathPacket{}
+
 func TestRealpathPacket(t *testing.T) {
 	const (
 		id   = 42
@@ -257,11 +244,10 @@ func TestRealpathPacket(t *testing.T) {
 	)
 
 	p := &RealpathPacket{
-		RequestID: id,
+		Path: path,
-		Path:      path,
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -279,20 +265,18 @@ func TestRealpathPacket(t *testing.T) {
 
 	*p = RealpathPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Path != path {
-		t.Errorf("UnmarshalBinary(): Path was %q, but expected %q", p.Path, path)
+		t.Errorf("UnmarshalPacketBody(): Path was %q, but expected %q", p.Path, path)
 	}
 }
 
+var _ Packet = &StatPacket{}
+
 func TestStatPacket(t *testing.T) {
 	const (
 		id   = 42
@@ -300,11 +284,10 @@ func TestStatPacket(t *testing.T) {
 	)
 
 	p := &StatPacket{
-		RequestID: id,
+		Path: path,
-		Path:      path,
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -322,20 +305,18 @@ func TestStatPacket(t *testing.T) {
 
 	*p = StatPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Path != path {
-		t.Errorf("UnmarshalBinary(): Path was %q, but expected %q", p.Path, path)
+		t.Errorf("UnmarshalPacketBody(): Path was %q, but expected %q", p.Path, path)
 	}
 }
 
+var _ Packet = &RenamePacket{}
+
 func TestRenamePacket(t *testing.T) {
 	const (
 		id      = 42
@@ -344,12 +325,11 @@ func TestRenamePacket(t *testing.T) {
 	)
 
 	p := &RenamePacket{
-		RequestID: id,
+		OldPath: oldpath,
-		OldPath:   oldpath,
+		NewPath: newpath,
-		NewPath:   newpath,
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -368,24 +348,22 @@ func TestRenamePacket(t *testing.T) {
 
 	*p = RenamePacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.OldPath != oldpath {
-		t.Errorf("UnmarshalBinary(): OldPath was %q, but expected %q", p.OldPath, oldpath)
+		t.Errorf("UnmarshalPacketBody(): OldPath was %q, but expected %q", p.OldPath, oldpath)
 	}
 
 	if p.NewPath != newpath {
-		t.Errorf("UnmarshalBinary(): NewPath was %q, but expected %q", p.NewPath, newpath)
+		t.Errorf("UnmarshalPacketBody(): NewPath was %q, but expected %q", p.NewPath, newpath)
 	}
 }
 
+var _ Packet = &ReadlinkPacket{}
+
 func TestReadlinkPacket(t *testing.T) {
 	const (
 		id   = 42
@@ -393,11 +371,10 @@ func TestReadlinkPacket(t *testing.T) {
 	)
 
 	p := &ReadlinkPacket{
-		RequestID: id,
+		Path: path,
-		Path:      path,
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -415,20 +392,18 @@ func TestReadlinkPacket(t *testing.T) {
 
 	*p = ReadlinkPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Path != path {
-		t.Errorf("UnmarshalBinary(): Path was %q, but expected %q", p.Path, path)
+		t.Errorf("UnmarshalPacketBody(): Path was %q, but expected %q", p.Path, path)
 	}
 }
 
+var _ Packet = &SymlinkPacket{}
+
 func TestSymlinkPacket(t *testing.T) {
 	const (
 		id         = 42
@@ -437,12 +412,11 @@ func TestSymlinkPacket(t *testing.T) {
 	)
 
 	p := &SymlinkPacket{
-		RequestID:  id,
 		LinkPath:   linkpath,
 		TargetPath: targetpath,
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -461,20 +435,16 @@ func TestSymlinkPacket(t *testing.T) {
 
 	*p = SymlinkPacket{}
 
-	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
+	// UnmarshalPacketBody assumes the (length, type, request-id) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.LinkPath != linkpath {
-		t.Errorf("UnmarshalBinary(): LinkPath was %q, but expected %q", p.LinkPath, linkpath)
+		t.Errorf("UnmarshalPacketBody(): LinkPath was %q, but expected %q", p.LinkPath, linkpath)
 	}
 
 	if p.TargetPath != targetpath {
-		t.Errorf("UnmarshalBinary(): TargetPath was %q, but expected %q", p.TargetPath, targetpath)
+		t.Errorf("UnmarshalPacketBody(): TargetPath was %q, but expected %q", p.TargetPath, targetpath)
 	}
 }

encoding/ssh/filexfer/raw_packet.go

@@ -1,5 +1,10 @@
 package filexfer
 
+import (
+	"fmt"
+	"io"
+)
+
 // RawPacket implements the general packet format from draft-ietf-secsh-filexfer-02
 //
 // Defined in https://tools.ietf.org/html/draft-ietf-secsh-filexfer-02#section-3
@@ -7,40 +12,31 @@ type RawPacket struct {
 	Type      PacketType
 	RequestID uint32
 
-	Payload []byte
+	Data Buffer
+}
+
+// Reset clears the pointers and reference-semantic variables of RawPacket,
+// making it suitable to be put into a sync.Pool.
+func (p *RawPacket) Reset() {
+	p.Data.Reset()
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *RawPacket) MarshalPacket() (header, payload []byte, err error) {
+//
-	b := NewMarshalBuffer(p.Type, p.RequestID, 0)
+// The internal p.RequestID is overridden by the reqid argument.
+func (p *RawPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
+	b := NewMarshalBuffer(p.Type, reqid, 0)
 
-	return b.Packet(p.Payload)
+	return b.Packet(p.Data.Bytes())
 }
 
 // MarshalBinary returns p as the binary encoding of p.
 func (p *RawPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
+	return ComposePacket(p.MarshalPacket(p.RequestID))
 }
 
-// UnmarshalPacketBody unmarshals the packet body from the given Buffer.
+// UnmarshalFrom decodes a RawPacket from the given Buffer into p.
-//
+func (p *RawPacket) UnmarshalFrom(buf *Buffer) error {
-// NOTE: To avoid extra allocations, UnmarshalPackBody aliases the underlying Buffer’s byte slice.
-func (p *RawPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	p.Payload = buf.Bytes()
-	return nil
-}
-
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-//
-// NOTE: To avoid extra allocations, UnmarshalBinary aliases the given byte slice.
-func (p *RawPacket) UnmarshalBinary(data []byte) error {
-	buf := NewBuffer(data)
-
 	typ, err := buf.ConsumeUint8()
 	if err != nil {
 		return err
@@ -48,5 +44,90 @@ func (p *RawPacket) UnmarshalBinary(data []byte) error {
 
 	p.Type = PacketType(typ)
 
-	return p.UnmarshalPacketBody(buf)
+	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
+		return err
+	}
+
+	p.Data = *buf
+	return nil
+}
+
+// UnmarshalBinary decodes a full raw packet out of the given data.
+// It is assumed that the uint32(length) has already been consumed to receive the data.
+//
+// NOTE: To avoid extra allocations, UnmarshalBinary aliases the given byte slice.
+func (p *RawPacket) UnmarshalBinary(data []byte) error {
+	return p.UnmarshalFrom(NewBuffer(data))
+}
+
+// ReadFrom reads a full raw packet out of the given reader.
+func (p *RawPacket) ReadFrom(r io.Reader, maxPacketLength uint32) error {
+	lb := make([]byte, 4)
+	if _, err := io.ReadFull(r, lb); err != nil {
+		return err
+	}
+
+	length := unmarshalUint32(lb)
+	if length < 1 {
+		return ErrShortPacket
+	}
+	if length > maxPacketLength {
+		return ErrLongPacket
+	}
+
+	b := make([]byte, length)
+	if _, err := io.ReadFull(r, b); err != nil {
+		return err
+	}
+
+	return p.UnmarshalBinary(b)
+}
+
+// RequestPacket decodes a full request packet from the internal Data based on the Type.
+func (p *RawPacket) RequestPacket() (reqid uint32, packet Packet, err error) {
+	switch p.Type {
+	case PacketTypeOpen:
+		packet = new(OpenPacket)
+	case PacketTypeClose:
+		packet = new(ClosePacket)
+	case PacketTypeRead:
+		packet = new(ReadPacket)
+	case PacketTypeWrite:
+		packet = new(WritePacket)
+	case PacketTypeLstat:
+		packet = new(LstatPacket)
+	case PacketTypeFstat:
+		packet = new(FstatPacket)
+	case PacketTypeSetstat:
+		packet = new(SetstatPacket)
+	case PacketTypeFsetstat:
+		packet = new(FsetstatPacket)
+	case PacketTypeOpendir:
+		packet = new(OpendirPacket)
+	case PacketTypeReaddir:
+		packet = new(ReaddirPacket)
+	case PacketTypeRemove:
+		packet = new(RemovePacket)
+	case PacketTypeMkdir:
+		packet = new(MkdirPacket)
+	case PacketTypeRmdir:
+		packet = new(RmdirPacket)
+	case PacketTypeRealpath:
+		packet = new(RealpathPacket)
+	case PacketTypeStat:
+		packet = new(StatPacket)
+	case PacketTypeRename:
+		packet = new(RenamePacket)
+	case PacketTypeReadlink:
+		packet = new(ReadlinkPacket)
+	case PacketTypeSymlink:
+		packet = new(SymlinkPacket)
+	case PacketTypeExtended:
+		packet = new(ExtendedPacket)
+	default:
+		return p.RequestID, nil, fmt.Errorf("unexpected packet request type: %v", p.Type)
+	}
+
+	packet.UnmarshalPacketBody(&p.Data)
+	return p.RequestID, packet, err
 }

encoding/ssh/filexfer/raw_packet_test.go

@@ -11,8 +11,8 @@ func TestRawPacket(t *testing.T) {
 	p := &RawPacket{
 		Type:      PacketTypeStat,
 		RequestID: uint32(id),
-		Payload: []byte{
+		Data: Buffer{
-			0x00, 0x00, 0x00, 0x03, 'f', 'o', 'o',
+			b: []byte{0x00, 0x00, 0x00, 0x03, 'f', 'o', 'o'},
 		},
 	}
 
@@ -51,7 +51,7 @@ func TestRawPacket(t *testing.T) {
 		0x00, 0x00, 0x00, 3, 'f', 'o', 'o',
 	}
 
-	if !bytes.Equal(p.Payload, want) {
+	if !bytes.Equal(p.Data.Bytes(), want) {
-		t.Errorf("RawPacket.UnmarshalBinary(): Payload was %X, but expected %X", p.Payload, want)
+		t.Errorf("RawPacket.UnmarshalBinary(): Data was %X, but expected %X", p.Data, want)
 	}
 }

encoding/ssh/filexfer/response_packets.go

@@ -4,18 +4,17 @@ package filexfer
 //
 // Specified in https://tools.ietf.org/html/draft-ietf-secsh-filexfer-02#section-7
 type StatusPacket struct {
-	RequestID    uint32
 	StatusCode   Status
 	ErrorMessage string
 	LanguageTag  string
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *StatusPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *StatusPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	// uint32(error/status code) + string(error message) + string(language tag)
 	size := 4 + 4 + len(p.ErrorMessage) + 4 + len(p.LanguageTag)
 
-	b := NewMarshalBuffer(PacketTypeStatus, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeStatus, reqid, size)
 
 	b.AppendUint32(uint32(p.StatusCode))
 	b.AppendString(p.ErrorMessage)
@@ -24,11 +23,6 @@ func (p *StatusPacket) MarshalPacket() (header, payload []byte, err error) {
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *StatusPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *StatusPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -49,41 +43,22 @@ func (p *StatusPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *StatusPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // HandlePacket defines the SSH_FXP_HANDLE packet.
 type HandlePacket struct {
-	RequestID uint32
+	Handle string
-	Handle    string
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *HandlePacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *HandlePacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 + len(p.Handle) // string(handle)
 
-	b := NewMarshalBuffer(PacketTypeHandle, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeHandle, reqid, size)
 
 	b.AppendString(p.Handle)
 
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *HandlePacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *HandlePacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -94,41 +69,22 @@ func (p *HandlePacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *HandlePacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // DataPacket defines the SSH_FXP_DATA packet.
 type DataPacket struct {
-	RequestID uint32
+	Data []byte
-	Data      []byte
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *DataPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *DataPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 // uint32(len(data)); data content in payload
 
-	b := NewMarshalBuffer(PacketTypeData, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeData, reqid, size)
 
 	b.AppendUint32(uint32(len(p.Data)))
 
 	return b.Packet(p.Data)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *DataPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *DataPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -139,34 +95,20 @@ func (p *DataPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *DataPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // NamePacket defines the SSH_FXP_NAME packet.
 type NamePacket struct {
-	RequestID uint32
+	Entries []*NameEntry
-	Entries   []*NameEntry
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *NamePacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *NamePacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := 4 // uint32(len(entries))
 
 	for _, e := range p.Entries {
 		size += e.Len()
 	}
 
-	b := NewMarshalBuffer(PacketTypeName, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeName, reqid, size)
 
 	b.AppendUint32(uint32(len(p.Entries)))
 
@@ -177,11 +119,6 @@ func (p *NamePacket) MarshalPacket() (header, payload []byte, err error) {
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *NamePacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *NamePacket) UnmarshalPacketBody(buf *Buffer) (err error) {
@@ -204,56 +141,24 @@ func (p *NamePacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return nil
 }
 
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *NamePacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}
-
 // AttrsPacket defines the SSH_FXP_ATTRS packet.
 type AttrsPacket struct {
-	RequestID uint32
+	Attrs Attributes
-	Attrs     Attributes
 }
 
 // MarshalPacket returns p as a two-part binary encoding of p.
-func (p *AttrsPacket) MarshalPacket() (header, payload []byte, err error) {
+func (p *AttrsPacket) MarshalPacket(reqid uint32) (header, payload []byte, err error) {
 	size := p.Attrs.Len() // ATTRS(attrs)
 
-	b := NewMarshalBuffer(PacketTypeAttrs, p.RequestID, size)
+	b := NewMarshalBuffer(PacketTypeAttrs, reqid, size)
 
 	p.Attrs.MarshalInto(b)
 
 	return b.Packet(payload)
 }
 
-// MarshalBinary returns p as the binary encoding of p.
-func (p *AttrsPacket) MarshalBinary() ([]byte, error) {
-	return ComposePacket(p.MarshalPacket())
-}
-
 // UnmarshalPacketBody unmarshals the packet body from the given Buffer.
 // It is assumed that the uint32(request-id) has already been consumed.
 func (p *AttrsPacket) UnmarshalPacketBody(buf *Buffer) (err error) {
 	return p.Attrs.UnmarshalFrom(buf)
 }
-
-// UnmarshalBinary unmarshals a full raw packet out of the given data.
-// It is assumed that the uint32(length) has already been consumed to receive the data.
-// It is also assumed that the uint8(type) has already been consumed to which packet to unmarshal into.
-func (p *AttrsPacket) UnmarshalBinary(data []byte) (err error) {
-	buf := NewBuffer(data)
-
-	if p.RequestID, err = buf.ConsumeUint32(); err != nil {
-		return err
-	}
-
-	return p.UnmarshalPacketBody(buf)
-}

encoding/ssh/filexfer/response_packets_test.go

@@ -5,6 +5,8 @@ import (
 	"testing"
 )
 
+var _ Packet = &StatusPacket{}
+
 func TestStatusPacket(t *testing.T) {
 	const (
 		id           = 42
@@ -14,13 +16,12 @@ func TestStatusPacket(t *testing.T) {
 	)
 
 	p := &StatusPacket{
-		RequestID:    id,
 		StatusCode:   statusCode,
 		ErrorMessage: errorMessage,
 		LanguageTag:  languageTag,
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -41,14 +42,10 @@ func TestStatusPacket(t *testing.T) {
 	*p = StatusPacket{}
 
 	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.StatusCode != statusCode {
 		t.Errorf("UnmarshalBinary(): StatusCode was %v, but expected %v", p.StatusCode, statusCode)
 	}
@@ -62,6 +59,8 @@ func TestStatusPacket(t *testing.T) {
 	}
 }
 
+var _ Packet = &HandlePacket{}
+
 func TestHandlePacket(t *testing.T) {
 	const (
 		id     = 42
@@ -69,11 +68,10 @@ func TestHandlePacket(t *testing.T) {
 	)
 
 	p := &HandlePacket{
-		RequestID: id,
+		Handle: "somehandle",
-		Handle:    "somehandle",
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -92,19 +90,17 @@ func TestHandlePacket(t *testing.T) {
 	*p = HandlePacket{}
 
 	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Handle != handle {
 		t.Errorf("UnmarshalBinary(): Handle was %q, but expected %q", p.Handle, handle)
 	}
 }
 
+var _ Packet = &DataPacket{}
+
 func TestDataPacket(t *testing.T) {
 	const (
 		id = 42
@@ -113,11 +109,10 @@ func TestDataPacket(t *testing.T) {
 	var payload = []byte(`foobar`)
 
 	p := &DataPacket{
-		RequestID: id,
+		Data: payload,
-		Data:      payload,
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -136,19 +131,17 @@ func TestDataPacket(t *testing.T) {
 	*p = DataPacket{}
 
 	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if !bytes.Equal(p.Data, payload) {
 		t.Errorf("UnmarshalBinary(): Data was %X, but expected %X", p.Data, payload)
 	}
 }
 
+var _ Packet = &NamePacket{}
+
 func TestNamePacket(t *testing.T) {
 	const (
 		id       = 42
@@ -158,7 +151,6 @@ func TestNamePacket(t *testing.T) {
 	)
 
 	p := &NamePacket{
-		RequestID: id,
 		Entries: []*NameEntry{
 			&NameEntry{
 				Filename: filename + "1",
@@ -179,7 +171,7 @@ func TestNamePacket(t *testing.T) {
 		},
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -206,14 +198,10 @@ func TestNamePacket(t *testing.T) {
 	*p = NamePacket{}
 
 	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if count := len(p.Entries); count != 2 {
 		t.Fatalf("UnmarshalBinary(): len(NameEntries) was %d, but expected %d", count, 2)
 	}
@@ -237,6 +225,8 @@ func TestNamePacket(t *testing.T) {
 	}
 }
 
+var _ Packet = &AttrsPacket{}
+
 func TestAttrsPacket(t *testing.T) {
 	const (
 		id    = 42
@@ -244,14 +234,13 @@ func TestAttrsPacket(t *testing.T) {
 	)
 
 	p := &AttrsPacket{
-		RequestID: id,
 		Attrs: Attributes{
 			Flags:       AttrPermissions,
 			Permissions: perms,
 		},
 	}
 
-	data, err := p.MarshalBinary()
+	data, err := ComposePacket(p.MarshalPacket(id))
 	if err != nil {
 		t.Fatal("unexpected error:", err)
 	}
@@ -271,14 +260,10 @@ func TestAttrsPacket(t *testing.T) {
 	*p = AttrsPacket{}
 
 	// UnmarshalBinary assumes the uint32(length) + uint8(type) have already been consumed.
-	if err := p.UnmarshalBinary(data[5:]); err != nil {
+	if err := p.UnmarshalPacketBody(NewBuffer(data[9:])); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 
-	if p.RequestID != uint32(id) {
-		t.Errorf("UnmarshalBinary(): RequestID was %d, but expected %d", p.RequestID, id)
-	}
-
 	if p.Attrs.Flags != AttrPermissions {
 		t.Errorf("UnmarshalBinary(): Attrs.Flags was %#x, but expected %#x", p.Attrs.Flags, AttrPermissions)
 	}