diff --git a/field/binary.go b/field/binary.go index e0000031..527714d6 100644 --- a/field/binary.go +++ b/field/binary.go @@ -66,10 +66,6 @@ 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) diff --git a/field/binary_test.go b/field/binary_test.go index 0a0f64ef..c37966ce 100644 --- a/field/binary_test.go +++ b/field/binary_test.go @@ -28,16 +28,6 @@ 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 @@ -119,4 +109,11 @@ func TestBinaryField(t *testing.T) { require.NoError(t, err) require.Equal(t, `"AB"`, string(marshalledJSON)) }) + + t.Run("returns error for zero value when fixed length and no padding specified", func(t *testing.T) { + bin := NewBinary(spec) + _, err := bin.Pack() + + require.EqualError(t, err, "failed to encode length: field length: 0 should be fixed: 10") + }) } diff --git a/field/composite.go b/field/composite.go index 72e735dc..09a673c4 100644 --- a/field/composite.go +++ b/field/composite.go @@ -238,10 +238,6 @@ 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) @@ -259,15 +255,10 @@ 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], hasPrefix) + read, err := f.unpack(data[offset : offset+dataLen]) if err != nil { return 0, err } @@ -283,7 +274,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, false) + _, err := f.unpack(data) return err } @@ -347,7 +338,6 @@ func (f *Composite) pack() ([]byte, error) { field, ok := f.subfields[tag] if !ok { return nil, fmt.Errorf("no subfield for tag %s", tag) - // continue } if _, set := f.setSubfields[tag]; !set { @@ -371,18 +361,20 @@ func (f *Composite) pack() ([]byte, error) { return nil, fmt.Errorf("failed to pack subfield %v: %w", tag, err) } packed = append(packed, packedBytes...) + } + return packed, nil } -func (f *Composite) unpack(data []byte, hasPrefix bool) (int, error) { +func (f *Composite) unpack(data []byte) (int, error) { if f.spec.Tag.Enc != nil { return f.unpackSubfieldsByTag(data) } - return f.unpackSubfields(data, hasPrefix) + return f.unpackSubfields(data) } -func (f *Composite) unpackSubfields(data []byte, hasPrefix bool) (int, error) { +func (f *Composite) unpackSubfields(data []byte) (int, error) { offset := 0 for _, tag := range f.orderedSpecFieldTags { field, ok := f.subfields[tag] @@ -398,11 +390,8 @@ func (f *Composite) unpackSubfields(data []byte, hasPrefix bool) (int, error) { f.setSubfields[tag] = struct{}{} offset += read - - if hasPrefix && offset >= len(data) { - break - } } + return offset, nil } diff --git a/field/composite_test.go b/field/composite_test.go index e3cd488c..84c203ec 100644 --- a/field/composite_test.go +++ b/field/composite_test.go @@ -296,7 +296,7 @@ func TestCompositePacking(t *testing.T) { require.EqualError(t, err, "failed to pack subfield 1: failed to encode length: field length: 4 should be fixed: 2") }) - t.Run("Pack returns error on failure to encode length", func(t *testing.T) { + t.Run("Pack returns error when encoded data length is larger than specified fixed max length", func(t *testing.T) { invalidSpec := &Spec{ // Base field length < summation of lengths of subfields // This will throw an error when encoding the field's length. @@ -330,7 +330,7 @@ func TestCompositePacking(t *testing.T) { } composite := NewComposite(invalidSpec) - err := composite.SetData(data) + err := composite.Marshal(data) require.NoError(t, err) _, err = composite.Pack() @@ -450,11 +450,8 @@ func TestCompositePacking(t *testing.T) { }), }, } - data := &CompositeTestData{} composite := NewComposite(invalidSpec) - err := composite.SetData(data) - require.NoError(t, err) // Length of input too long, causing failure to decode length. read, err := composite.Unpack([]byte("ABCD123")) @@ -496,7 +493,7 @@ func TestCompositePacking(t *testing.T) { } func TestCompositePackingWithTags(t *testing.T) { - t.Run("Pack returns error on failure to encode length", func(t *testing.T) { + t.Run("Pack returns error when encoded data length is larger than specified fixed max length", func(t *testing.T) { // Base field length < summation of (lengths of subfields + IDs). // This will throw an error when encoding the field's length. invalidSpec := &Spec{ @@ -542,6 +539,49 @@ func TestCompositePackingWithTags(t *testing.T) { require.EqualError(t, err, "failed to encode length: field length: 12 should be fixed: 6") }) + t.Run("Pack returns error when encoded data length is larger than specified variable max length", func(t *testing.T) { + invalidSpec := &Spec{ + Length: 8, + Pref: prefix.ASCII.LL, + Tag: &TagSpec{ + Length: 2, + Enc: encoding.ASCII, + Pad: padding.Left('0'), + Sort: sort.StringsByInt, + }, + Subfields: map[string]Field{ + "1": NewString(&Spec{ + Length: 2, + Enc: encoding.ASCII, + Pref: prefix.ASCII.Fixed, + }), + "2": NewString(&Spec{ + Length: 2, + Enc: encoding.ASCII, + Pref: prefix.ASCII.Fixed, + }), + "3": NewNumeric(&Spec{ + Length: 2, + Enc: encoding.ASCII, + Pref: prefix.ASCII.Fixed, + }), + }, + } + data := &CompositeTestData{ + F1: NewStringValue("AB"), + F2: NewStringValue("CD"), + F3: NewNumericValue(12), + } + + composite := NewComposite(invalidSpec) + err := composite.Marshal(data) + require.NoError(t, err) + + b, err := composite.Pack() + require.Nil(t, b) + require.EqualError(t, err, "failed to encode length: field length: 12 is larger than maximum: 8") + }) + t.Run("Pack correctly serializes fully populated data to bytes", func(t *testing.T) { data := &CompositeTestData{ F1: NewStringValue("AB"), @@ -595,142 +635,6 @@ 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{} diff --git a/field/field.go b/field/field.go index e0550f32..306153fd 100644 --- a/field/field.go +++ b/field/field.go @@ -30,7 +30,7 @@ type Field interface { // a pointer it returns error. Unmarshal(v interface{}) error - // Marshal sets field Value into provided v. If v is nil or not + // Marshal sets field Value from provided v. If v is nil or not // a pointer it returns error. Marshal(v interface{}) error diff --git a/field/numeric_test.go b/field/numeric_test.go index 7f6c9008..79d5c3e7 100644 --- a/field/numeric_test.go +++ b/field/numeric_test.go @@ -51,6 +51,22 @@ func TestNumericField(t *testing.T) { require.Equal(t, 9876, data.Value) } +func TestNumericPack(t *testing.T) { + t.Run("returns error for zero value when fixed length and no padding specified", func(t *testing.T) { + spec := &Spec{ + Length: 10, + Description: "Field", + Enc: encoding.ASCII, + Pref: prefix.ASCII.Fixed, + } + numeric := NewNumeric(spec) + _, err := numeric.Pack() + + // zero value for Numeric is 0, so we have default field length 1 + require.EqualError(t, err, "failed to encode length: field length: 1 should be fixed: 10") + }) +} + func TestNumericFieldUnmarshal(t *testing.T) { str := NewNumericValue(9876) diff --git a/field/string.go b/field/string.go index d65c7349..53238d9b 100644 --- a/field/string.go +++ b/field/string.go @@ -64,10 +64,6 @@ 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) diff --git a/field/string_test.go b/field/string_test.go index 72623874..eb83f384 100644 --- a/field/string_test.go +++ b/field/string_test.go @@ -56,6 +56,22 @@ func TestStringField(t *testing.T) { err = str.SetBytes([]byte("hello")) require.NoError(t, err) require.Equal(t, "hello", data.Value) + +} + +func TestStringPack(t *testing.T) { + t.Run("returns error for zero value when fixed length and no padding specified", func(t *testing.T) { + spec := &Spec{ + Length: 10, + Description: "Field", + Enc: encoding.ASCII, + Pref: prefix.ASCII.Fixed, + } + str := NewString(spec) + _, err := str.Pack() + + require.EqualError(t, err, "failed to encode length: field length: 0 should be fixed: 10") + }) } func TestStringFieldUnmarshal(t *testing.T) { @@ -69,21 +85,6 @@ func TestStringFieldUnmarshal(t *testing.T) { require.Equal(t, "hello", val.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"`) diff --git a/field/track1.go b/field/track1.go index 917e1c10..5f82b542 100644 --- a/field/track1.go +++ b/field/track1.go @@ -77,10 +77,6 @@ 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) diff --git a/field/track2.go b/field/track2.go index f4870e5e..5ce87ca8 100644 --- a/field/track2.go +++ b/field/track2.go @@ -76,10 +76,6 @@ 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) diff --git a/field/track3.go b/field/track3.go index 4f0a7523..83c1f10b 100644 --- a/field/track3.go +++ b/field/track3.go @@ -74,10 +74,6 @@ 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)