Merge pull request #438 from pkg/patch/bugfixes-to-filexfer

Pre-Merge filexfer Bugfixes

Author: puellanivis

internal/encoding/ssh/filexfer/attrs.go

@@ -34,6 +34,57 @@ type Attributes struct {
 	ExtendedAttributes []ExtendedAttribute
 }
 
+// GetSize returns the Size field and a bool that is true if and only if the value is valid/defined.
+func (a *Attributes) GetSize() (size uint64, ok bool) {
+	return a.Size, a.Flags&AttrSize != 0
+}
+
+// SetSize is a convenience function that sets the Size field,
+// and marks the field as valid/defined in Flags.
+func (a *Attributes) SetSize(size uint64) {
+	a.Flags |= AttrSize
+	a.Size = size
+}
+
+// GetUIDGID returns the UID and GID fields and a bool that is true if and only if the values are valid/defined.
+func (a *Attributes) GetUIDGID() (uid, gid uint32, ok bool) {
+	return a.UID, a.GID, a.Flags&AttrUIDGID != 0
+}
+
+// SetUIDGID is a convenience function that sets the UID and GID fields,
+// and marks the fields as valid/defined in Flags.
+func (a *Attributes) SetUIDGID(uid, gid uint32) {
+	a.Flags |= AttrUIDGID
+	a.UID = uid
+	a.GID = gid
+}
+
+// GetPermissions returns the Permissions field and a bool that is true if and only if the value is valid/defined.
+func (a *Attributes) GetPermissions() (perms FileMode, ok bool) {
+	return a.Permissions, a.Flags&AttrPermissions != 0
+}
+
+// SetPermissions is a convenience function that sets the Permissions field,
+// and marks the field as valid/defined in Flags.
+func (a *Attributes) SetPermissions(perms FileMode) {
+	a.Flags |= AttrPermissions
+	a.Permissions = perms
+}
+
+// GetACModTime returns the ATime and MTime fields and a bool that is true if and only if the values are valid/defined.
+func (a *Attributes) GetACModTime() (atime, mtime uint32, ok bool) {
+	return a.ATime, a.MTime, a.Flags&AttrACModTime != 0
+	return a.ATime, a.MTime, a.Flags&AttrACModTime != 0
+}
+
+// SetACModTime is a convenience function that sets the ATime and MTime fields,
+// and marks the fields as valid/defined in Flags.
+func (a *Attributes) SetACModTime(atime, mtime uint32) {
+	a.Flags |= AttrACModTime
+	a.ATime = atime
+	a.MTime = mtime
+}
+
 // Len returns the number of bytes a would marshal into.
 func (a *Attributes) Len() int {
 	length := 4
@@ -98,7 +149,7 @@ func (a *Attributes) MarshalInto(b *Buffer) {
 
 // MarshalBinary returns a as the binary encoding of a.
 func (a *Attributes) MarshalBinary() ([]byte, error) {
-	buf := NewBuffer(make([]byte, a.Len()))
+	buf := NewBuffer(make([]byte, 0, a.Len()))
 	a.MarshalInto(buf)
 	return buf.Bytes(), nil
 }
@@ -107,10 +158,20 @@ func (a *Attributes) MarshalBinary() ([]byte, error) {
 //
 // NOTE: The values of fields not covered in the a.Flags are explicitly undefined.
 func (a *Attributes) UnmarshalFrom(b *Buffer) (err error) {
-	if a.Flags, err = b.ConsumeUint32(); err != nil {
+	flags, err := b.ConsumeUint32()
+	if err != nil {
 		return err
 	}
 
+	return a.XXX_UnmarshalByFlags(flags, b)
+}
+
+// XXX_UnmarshalByFlags uses the pre-existing a.Flags field to determine which fields to decode.
+// DO NOT USE THIS: it is an anti-corruption function to implement existing internal usage in pkg/sftp.
+// This function is not a part of any compatibility promise.
+func (a *Attributes) XXX_UnmarshalByFlags(flags uint32, b *Buffer) (err error) {
+	a.Flags = flags
+
 	// Short-circuit dummy attributes.
 	if a.Flags == 0 {
 		return nil
@@ -192,7 +253,7 @@ func (e *ExtendedAttribute) MarshalInto(b *Buffer) {
 
 // MarshalBinary returns e as the binary encoding of e.
 func (e *ExtendedAttribute) MarshalBinary() ([]byte, error) {
-	buf := NewBuffer(make([]byte, e.Len()))
+	buf := NewBuffer(make([]byte, 0, e.Len()))
 	e.MarshalInto(buf)
 	return buf.Bytes(), nil
 }
@@ -239,7 +300,7 @@ func (e *NameEntry) MarshalInto(b *Buffer) {
 
 // MarshalBinary returns e as the binary encoding of e.
 func (e *NameEntry) MarshalBinary() ([]byte, error) {
-	buf := NewBuffer(make([]byte, e.Len()))
+	buf := NewBuffer(make([]byte, 0, e.Len()))
 	e.MarshalInto(buf)
 	return buf.Bytes(), nil
 }

internal/encoding/ssh/filexfer/attrs_test.go

@@ -113,60 +113,62 @@ func TestAttributes(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			attr.Flags = tt.flags
 
-			buf := new(Buffer)
-			attr.MarshalInto(buf)
+			buf, err := attr.MarshalBinary()
+			if err != nil {
+				t.Fatal("unexpected error:", err)
+			}
 
-			if got, want := buf.Bytes(), tt.encoded; !bytes.Equal(got, want) {
-				t.Fatalf("MarshalInto() = %X, but wanted %X", got, want)
+			if !bytes.Equal(buf, tt.encoded) {
+				t.Fatalf("MarshalBinary() = %X, but wanted %X", buf, tt.encoded)
 			}
 
 			attr = Attributes{}
 
-			if err := attr.UnmarshalFrom(buf); err != nil {
+			if err := attr.UnmarshalBinary(buf); err != nil {
 				t.Fatal("unexpected error:", err)
 			}
 
 			if attr.Flags != tt.flags {
-				t.Errorf("UnmarshalFrom(): Flags was %x, but wanted %x", attr.Flags, tt.flags)
+				t.Errorf("UnmarshalBinary(): Flags was %x, but wanted %x", attr.Flags, tt.flags)
 			}
 
 			if attr.Flags&AttrSize != 0 && attr.Size != size {
-				t.Errorf("UnmarshalFrom(): Size was %x, but wanted %x", attr.Size, size)
+				t.Errorf("UnmarshalBinary(): Size was %x, but wanted %x", attr.Size, size)
 			}
 
 			if attr.Flags&AttrUIDGID != 0 {
 				if attr.UID != uid {
-					t.Errorf("UnmarshalFrom(): UID was %x, but wanted %x", attr.UID, uid)
+					t.Errorf("UnmarshalBinary(): UID was %x, but wanted %x", attr.UID, uid)
 				}
 
 				if attr.GID != gid {
-					t.Errorf("UnmarshalFrom(): GID was %x, but wanted %x", attr.GID, gid)
+					t.Errorf("UnmarshalBinary(): GID was %x, but wanted %x", attr.GID, gid)
 				}
 			}
 
 			if attr.Flags&AttrPermissions != 0 && attr.Permissions != perms {
-				t.Errorf("UnmarshalFrom(): Permissions was %#v, but wanted %#v", attr.Permissions, perms)
+				t.Errorf("UnmarshalBinary(): Permissions was %#v, but wanted %#v", attr.Permissions, perms)
 			}
 
 			if attr.Flags&AttrACModTime != 0 {
 				if attr.ATime != atime {
-					t.Errorf("UnmarshalFrom(): ATime was %x, but wanted %x", attr.ATime, atime)
+					t.Errorf("UnmarshalBinary(): ATime was %x, but wanted %x", attr.ATime, atime)
 				}
 
 				if attr.MTime != mtime {
-					t.Errorf("UnmarshalFrom(): MTime was %x, but wanted %x", attr.MTime, mtime)
+					t.Errorf("UnmarshalBinary(): MTime was %x, but wanted %x", attr.MTime, mtime)
 				}
 			}
 
 			if attr.Flags&AttrExtended != 0 {
 				extAttrs := attr.ExtendedAttributes
 
 				if count := len(extAttrs); count != 1 {
-					t.Fatalf("UnmarshalFrom(): len(ExtendedAttributes) was %d, but wanted %d", count, 1)
+					t.Fatalf("UnmarshalBinary(): len(ExtendedAttributes) was %d, but wanted %d", count, 1)
 				}
 
 				if got := extAttrs[0]; got != extAttr {
-					t.Errorf("UnmarshalFrom(): ExtendedAttributes[0] was %#v, but wanted %#v", got, extAttr)
+					t.Errorf("UnmarshalBinary(): ExtendedAttributes[0] was %#v, but wanted %#v", got, extAttr)
 				}
 			}
 		})
@@ -189,8 +191,10 @@ func TestNameEntry(t *testing.T) {
 		},
 	}
 
-	buf := new(Buffer)
-	e.MarshalInto(buf)
+	buf, err := e.MarshalBinary()
+	if err != nil {
+		t.Fatal("unexpected error:", err)
+	}
 
 	want := []byte{
 		0x00, 0x00, 0x00, 0x03, 'f', 'o', 'o',
@@ -199,13 +203,13 @@ func TestNameEntry(t *testing.T) {
 		0x87, 0x65, 0x43, 0x21,
 	}
 
-	if got := buf.Bytes(); !bytes.Equal(got, want) {
-		t.Fatalf("MarshalInto() = %X, but wanted %X", got, want)
+	if !bytes.Equal(buf, want) {
+		t.Fatalf("MarshalBinary() = %X, but wanted %X", buf, want)
 	}
 
 	*e = NameEntry{}
 
-	if err := e.UnmarshalFrom(buf); err != nil {
+	if err := e.UnmarshalBinary(buf); err != nil {
 		t.Fatal("unexpected error:", err)
 	}
 

internal/encoding/ssh/filexfer/extended_packets.go

@@ -51,6 +51,11 @@ type ExtendedPacket struct {
 	Data ExtendedData
 }
 
+// Type returns the SSH_FXP_xy value associated with this packet type.
+func (p *ExtendedPacket) Type() PacketType {
+	return PacketTypeExtended
+}
+
 // MarshalPacket returns p as a two-part binary encoding of p.
 //
 // The Data is marshaled into binary, and returned as the payload.
@@ -97,6 +102,11 @@ type ExtendedReplyPacket struct {
 	Data ExtendedData
 }
 
+// Type returns the SSH_FXP_xy value associated with this packet type.
+func (p *ExtendedReplyPacket) Type() PacketType {
+	return PacketTypeExtendedReply
+}
+
 // MarshalPacket returns p as a two-part binary encoding of p.
 //
 // The Data is marshaled into binary, and returned as the payload.