Return early when packing zero length fields (#149)

* Ignore zero length fields

* add flag that allows parsing of prefixed composites to only up to specified length

* remove debug line

* add tests for zero length fields

Co-authored-by: dakinola <dakinola@thoughtmachine.net>

field/binary.go

@@ -65,6 +65,10 @@ func (f *Binary) Pack() ([]byte, error) {
 		return nil, fmt.Errorf("failed to encode content: %w", err)
 	}
 
+	if len(packed) == 0 {
+		return []byte{}, nil
+	}
+
 	packedLength, err := f.spec.Pref.EncodeLength(f.spec.Length, len(packed))
 	if err != nil {
 		return nil, fmt.Errorf("failed to encode length: %w", err)

field/binary_test.go

@@ -28,6 +28,16 @@ func TestBinaryField(t *testing.T) {
 		require.Equal(t, in, packed)
 	})
 
+	t.Run("Pack returns empty struct when given zero-length data", func(t *testing.T) {
+		bin := NewBinaryValue([]byte{})
+		bin.SetSpec(spec)
+
+		packed, err := bin.Pack()
+
+		require.NoError(t, err)
+		require.Equal(t, []byte{}, packed)
+	})
+
 	t.Run("String returns binary data encoded in HEX", func(t *testing.T) {
 		bin := NewBinary(spec)
 		bin.Value = in

field/composite.go

@@ -125,6 +125,10 @@ func (f *Composite) Pack() ([]byte, error) {
 		return nil, err
 	}
 
+	if len(packed) == 0 {
+		return []byte{}, nil
+	}
+
 	packedLength, err := f.spec.Pref.EncodeLength(f.spec.Length, len(packed))
 	if err != nil {
 		return nil, fmt.Errorf("failed to encode length: %w", err)
@@ -142,10 +146,15 @@ func (f *Composite) Unpack(data []byte) (int, error) {
 		return 0, fmt.Errorf("failed to decode length: %w", err)
 	}
 
+	hasPrefix := false
+	if offset != 0 {
+		hasPrefix = true
+	}
+
 	// data is stripped of the prefix before it is provided to unpack().
 	// Therefore, it is unaware of when to stop parsing unless we bound the
 	// length of the slice by the data length.
-	read, err := f.unpack(data[offset : offset+dataLen])
+	read, err := f.unpack(data[offset:offset+dataLen], hasPrefix)
 	if err != nil {
 		return 0, err
 	}
@@ -161,7 +170,7 @@ func (f *Composite) Unpack(data []byte) (int, error) {
 // pack all subfields in full. However, unlike Unpack(), it requires the
 // aggregate length of the subfields not to be encoded in the prefix.
 func (f *Composite) SetBytes(data []byte) error {
-	_, err := f.unpack(data)
+	_, err := f.unpack(data, false)
 	return err
 }
 
@@ -198,15 +207,18 @@ func (f *Composite) MarshalJSON() ([]byte, error) {
 // been defined in the spec.
 func (f *Composite) UnmarshalJSON(b []byte) error {
 	var data map[string]json.RawMessage
-	json.Unmarshal(b, &data)
+	err := json.Unmarshal(b, &data)
+	if err != nil {
+		return err
+	}
 
 	for tag, rawMsg := range data {
 		if _, ok := f.spec.Subfields[tag]; !ok {
 			return fmt.Errorf("failed to unmarshal subfield %v: received subfield not defined in spec", tag)
 		}
 		subfield := f.createAndSetSubfield(tag)
 
-		if err := f.setSubfieldData(tag, subfield); err != nil {
+		if err = f.setSubfieldData(tag, subfield); err != nil {
 			return err
 		}
 		if err := json.Unmarshal(rawMsg, subfield); err != nil {
@@ -264,14 +276,14 @@ func (f *Composite) pack() ([]byte, error) {
 	return packed, nil
 }
 
-func (f *Composite) unpack(data []byte) (int, error) {
+func (f *Composite) unpack(data []byte, hasPrefix bool) (int, error) {
 	if f.spec.Tag.Enc != nil {
 		return f.unpackSubfieldsByTag(data)
 	}
-	return f.unpackSubfields(data)
+	return f.unpackSubfields(data, hasPrefix)
 }
 
-func (f *Composite) unpackSubfields(data []byte) (int, error) {
+func (f *Composite) unpackSubfields(data []byte, hasPrefix bool) (int, error) {
 	offset := 0
 	for _, tag := range f.orderedSpecFieldTags {
 		field := f.createAndSetSubfield(tag)
@@ -283,6 +295,10 @@ func (f *Composite) unpackSubfields(data []byte) (int, error) {
 			return 0, fmt.Errorf("failed to unpack subfield %v: %w", tag, err)
 		}
 		offset += read
+
+		if hasPrefix && offset >= len(data) {
+			break
+		}
 	}
 	return offset, nil
 }

field/composite_test.go

@@ -543,6 +543,142 @@ func TestCompositePackingWithTags(t *testing.T) {
 		require.Equal(t, "120102AB0202CD", string(packed))
 	})
 
+	t.Run("Pack correctly ignores excess subfields in excess of the length described by the prefix", func(t *testing.T) {
+		type ExcessSubfieldsTestData struct {
+			F1  *String
+			F2  *Numeric
+			F3  *String
+			F4  *String
+			F5  *String
+			F6  *String
+			F7  *String
+			F8  *String
+			F9  *Numeric
+			F10 *String
+			F11 *String
+			F12 *String
+			F13 *String
+			F14 *String
+		}
+
+		excessSubfieldsSpec := &Spec{
+			Length:      26,
+			Description: "POS Data",
+			Pref:        prefix.ASCII.LLL,
+			Tag: &TagSpec{
+				Sort: sort.StringsByInt,
+			},
+			Subfields: map[string]Field{
+				"1": NewString(&Spec{
+					Length:      1,
+					Description: "POS Terminal Attendance",
+					Enc:         encoding.ASCII,
+					Pref:        prefix.ASCII.Fixed,
+				}),
+				"2": NewNumeric(&Spec{
+					Length:      1,
+					Description: "Reserved for Future Use",
+					Enc:         encoding.ASCII,
+					Pref:        prefix.ASCII.Fixed,
+				}),
+				"3": NewString(&Spec{
+					Length:      1,
+					Description: "POS Terminal Location",
+					Enc:         encoding.ASCII,
+					Pref:        prefix.ASCII.Fixed,
+				}),
+				"4": NewString(&Spec{
+					Length:      1,
+					Description: "POS Cardholder Presence",
+					Enc:         encoding.ASCII,
+					Pref:        prefix.ASCII.Fixed,
+				}),
+				"5": NewString(&Spec{
+					Length:      1,
+					Description: "POS Card Presence",
+					Enc:         encoding.ASCII,
+					Pref:        prefix.ASCII.Fixed,
+				}),
+				"6": NewString(&Spec{
+					Length:      1,
+					Description: "POS Card Capture Capabilities",
+					Enc:         encoding.ASCII,
+					Pref:        prefix.ASCII.Fixed,
+				}),
+				"7": NewString(&Spec{
+					Length:      1,
+					Description: "POS Transaction Status",
+					Enc:         encoding.ASCII,
+					Pref:        prefix.ASCII.Fixed,
+				}),
+				"8": NewString(&Spec{
+					Length:      1,
+					Description: "POS Transaction Security",
+					Enc:         encoding.ASCII,
+					Pref:        prefix.ASCII.Fixed,
+				}),
+				"9": NewNumeric(&Spec{
+					Length:      1,
+					Description: "Reserved for Future Use",
+					Enc:         encoding.ASCII,
+					Pref:        prefix.ASCII.Fixed,
+					Pad:         padding.Left('0'),
+				}),
+				"10": NewString(&Spec{
+					Length:      1,
+					Description: "Cardholder-Activated Terminal Level",
+					Enc:         encoding.ASCII,
+					Pref:        prefix.ASCII.Fixed,
+				}),
+				"11": NewString(&Spec{
+					Length:      1,
+					Description: "POS Card Data Terminal Input Capability Indicator",
+					Enc:         encoding.ASCII,
+					Pref:        prefix.ASCII.Fixed,
+				}),
+				"12": NewString(&Spec{
+					Length:      2,
+					Description: "POS Authorization Life Cycle",
+					Enc:         encoding.ASCII,
+					Pref:        prefix.ASCII.Fixed,
+				}),
+				"13": NewString(&Spec{
+					Length:      3,
+					Description: "POS Country Code",
+					Enc:         encoding.ASCII,
+					Pref:        prefix.ASCII.Fixed,
+				}),
+				"14": NewString(&Spec{
+					Length:      10,
+					Description: "POS Postal Code",
+					Enc:         encoding.ASCII,
+					Pref:        prefix.ASCII.Fixed,
+				}),
+			},
+		}
+
+		data := &ExcessSubfieldsTestData{}
+
+		composite := NewComposite(excessSubfieldsSpec)
+		err := composite.SetData(data)
+		require.NoError(t, err)
+
+		// Subfield 12, 13 & 14 fall outside of the bounds of the
+		// 11 byte limit imposed by the prefix. [011 | 10000100012]
+		// Therefore, it won't be included in the packed bytes.
+
+		packed := []byte("01110000100012")
+
+		read, err := composite.Unpack(packed)
+
+		require.NoError(t, err)
+		require.Equal(t, 14, read)
+
+		packedBytes, err := composite.Pack()
+		require.NoError(t, err)
+		require.Equal(t, packedBytes, packed)
+	})
+
 	t.Run("Unpack returns an error on failure of subfield to unpack bytes", func(t *testing.T) {
 		data := &CompositeTestData{}
 

field/string.go

@@ -63,6 +63,10 @@ func (f *String) Pack() ([]byte, error) {
 		return nil, fmt.Errorf("failed to encode content: %w", err)
 	}
 
+	if len(packed) == 0 {
+		return []byte{}, nil
+	}
+
 	packedLength, err := f.spec.Pref.EncodeLength(f.spec.Length, len(packed))
 	if err != nil {
 		return nil, fmt.Errorf("failed to encode length: %w", err)

field/string_test.go

@@ -58,6 +58,21 @@ func TestStringField(t *testing.T) {
 	require.Equal(t, "hello", data.Value)
 }
 
+func TestStringFieldZeroLength(t *testing.T) {
+	str := NewStringValue("")
+	str.SetSpec(&Spec{
+		Length:      10,
+		Description: "Field",
+		Enc:         encoding.ASCII,
+		Pref:        prefix.ASCII.Fixed,
+	})
+
+	packed, err := str.Pack()
+	require.NoError(t, err)
+	require.Equal(t, []byte{}, packed)
+	require.Equal(t, "", string(packed))
+}
+
 func TestStringJSONUnmarshal(t *testing.T) {
 	input := []byte(`"4000"`)
 

field/track1.go

@@ -77,6 +77,10 @@ func (f *Track1) Pack() ([]byte, error) {
 		return nil, fmt.Errorf("failed to encode content: %w", err)
 	}
 
+	if len(packed) == 0 {
+		return []byte{}, nil
+	}
+
 	packedLength, err := f.spec.Pref.EncodeLength(f.spec.Length, len(packed))
 	if err != nil {
 		return nil, fmt.Errorf("failed to encode length: %w", err)

field/track2.go

@@ -76,6 +76,10 @@ func (f *Track2) Pack() ([]byte, error) {
 		return nil, fmt.Errorf("failed to encode content: %w", err)
 	}
 
+	if len(packed) == 0 {
+		return []byte{}, nil
+	}
+
 	packedLength, err := f.spec.Pref.EncodeLength(f.spec.Length, len(packed))
 	if err != nil {
 		return nil, fmt.Errorf("failed to encode length: %w", err)

field/track3.go

@@ -74,6 +74,10 @@ func (f *Track3) Pack() ([]byte, error) {
 		return nil, fmt.Errorf("failed to encode content: %w", err)
 	}
 
+	if len(packed) == 0 {
+		return []byte{}, nil
+	}
+
 	packedLength, err := f.spec.Pref.EncodeLength(f.spec.Length, len(packed))
 	if err != nil {
 		return nil, fmt.Errorf("failed to encode length: %w", err)

go.mod

@@ -7,5 +7,6 @@ require (
 	github.com/kr/pretty v0.1.0 // indirect
 	github.com/stretchr/testify v1.7.0
 	github.com/yerden/go-util v1.1.4
+	golang.org/x/text v0.3.7
 	gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
 )

go.sum

@@ -15,6 +15,9 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
 github.com/yerden/go-util v1.1.4 h1:jd8JyjLHzpEs1ZZQzDkfRgosDtXp/BtIAV1kpNjVTtw=
 github.com/yerden/go-util v1.1.4/go.mod h1:3HeLrvtkEeAv67ARostM9Yn0DcAVqgJ3uAiCuywEEXk=
 golang.org/x/sys v0.0.0-20190913121621-c3b328c6e5a7/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/text v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk=
+golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
+golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo=
 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

padding/right.go

@@ -0,0 +1,43 @@
+package padding
+
+import (
+	"bytes"
+	"unicode/utf8"
+)
+
+// Right returns a new right-side padder
+var Right func(pad rune) Padder = NewRightPadder
+
+type rightPadder struct {
+	pad []byte
+}
+
+// NewRightPadder takes the given byte character and returns a padder
+// which pads fields to the right of their values (for left-justified values)
+func NewRightPadder(pad rune) Padder {
+	buf := make([]byte, utf8.RuneLen(pad))
+	utf8.EncodeRune(buf, pad)
+
+	return &rightPadder{buf}
+}
+
+func (p *rightPadder) Pad(data []byte, length int) []byte {
+	if len(data) >= length {
+		return data
+	}
+
+	padding := bytes.Repeat(p.pad, length-len(data))
+	return append(data, padding...)
+}
+
+func (p *rightPadder) Unpad(data []byte) []byte {
+	pad, _ := utf8.DecodeRune(p.pad)
+
+	return bytes.TrimRightFunc(data, func(r rune) bool {
+		return r == pad
+	})
+}
+
+func (p *rightPadder) Inspect() []byte {
+	return p.pad
+}