diff --git a/errors.go b/errors.go index 6aea3a3..f6a74fe 100644 --- a/errors.go +++ b/errors.go @@ -4,7 +4,7 @@ package iso8583 // connection failed to unpack message type UnpackError struct { Err error - Fields []string + Field string RawMessage []byte } @@ -17,8 +17,7 @@ func (e *UnpackError) Unwrap() error { } type PackError struct { - Fields []string - Err error + Err error } func (e *PackError) Error() string { diff --git a/field/numeric.go b/field/numeric.go index a3c485c..1c0f854 100644 --- a/field/numeric.go +++ b/field/numeric.go @@ -110,7 +110,7 @@ func (f *Numeric) Unpack(data []byte) (int, error) { raw, read, err := f.spec.Enc.Decode(data[prefBytes:], dataLen) if err != nil { - return dataLen, fmt.Errorf("failed to decode content: %w", err) + return 0, fmt.Errorf("failed to decode content: %w", err) } if f.spec.Pad != nil { @@ -118,7 +118,7 @@ func (f *Numeric) Unpack(data []byte) (int, error) { } if err := f.SetBytes(raw); err != nil { - return dataLen, fmt.Errorf("failed to set bytes: %w", err) + return 0, fmt.Errorf("failed to set bytes: %w", err) } return read + prefBytes, nil diff --git a/field/string.go b/field/string.go index 0819460..c8ce11c 100644 --- a/field/string.go +++ b/field/string.go @@ -97,7 +97,7 @@ func (f *String) Unpack(data []byte) (int, error) { raw, read, err := f.spec.Enc.Decode(data[prefBytes:], dataLen) if err != nil { - return dataLen, fmt.Errorf("failed to decode content: %w", err) + return 0, fmt.Errorf("failed to decode content: %w", err) } if f.spec.Pad != nil { @@ -105,7 +105,7 @@ func (f *String) Unpack(data []byte) (int, error) { } if err := f.SetBytes(raw); err != nil { - return dataLen, fmt.Errorf("failed to set bytes: %w", err) + return 0, fmt.Errorf("failed to set bytes: %w", err) } return read + prefBytes, nil diff --git a/message.go b/message.go index 6ec996a..329a074 100644 --- a/message.go +++ b/message.go @@ -7,7 +7,6 @@ import ( "reflect" "sort" "strconv" - "strings" "sync" "github.com/moov-io/iso8583/field" @@ -238,10 +237,10 @@ func (m *Message) Unpack(src []byte) error { // errors in a *UnpackError. It assumes that the mutex is already // locked by the caller. func (m *Message) wrappErrorUnpack(src []byte) error { - if fields, err := m.unpack(src); err != nil { + if field, err := m.unpack(src); err != nil { return &UnpackError{ Err: err, - Fields: fields, + Field: field, RawMessage: src, } } @@ -251,10 +250,8 @@ func (m *Message) wrappErrorUnpack(src []byte) error { // unpack contains the core logic for unpacking the message. This method does // not handle locking or error wrapping and should typically be used internally // after ensuring concurrency safety. -func (m *Message) unpack(src []byte) ([]string, error) { +func (m *Message) unpack(src []byte) (string, error) { var off int - fields := []string{} - var fieldError error // reset fields that were set m.fieldsMap = map[int]struct{}{} @@ -264,7 +261,7 @@ func (m *Message) unpack(src []byte) ([]string, error) { read, err := m.fields[mtiIdx].Unpack(src) if err != nil { - return nil, fmt.Errorf("failed to unpack MTI: %w", err) + return "", fmt.Errorf("failed to unpack MTI: %w", err) } m.fieldsMap[mtiIdx] = struct{}{} @@ -274,7 +271,7 @@ func (m *Message) unpack(src []byte) ([]string, error) { // unpack Bitmap read, err = m.fields[bitmapIdx].Unpack(src[off:]) if err != nil { - return nil, fmt.Errorf("failed to unpack bitmap: %w", err) + return "", fmt.Errorf("failed to unpack bitmap: %w", err) } off += read @@ -288,18 +285,12 @@ func (m *Message) unpack(src []byte) ([]string, error) { if m.bitmap().IsSet(i) { fl, ok := m.fields[i] if !ok { - fieldError = fmt.Errorf("failed to unpack field %d: no specification found", i) - fields = append(fields, strconv.Itoa(i)) - return fields, fieldError + return strconv.Itoa(i), fmt.Errorf("failed to unpack field %d: no specification found", i) } read, err = fl.Unpack(src[off:]) if err != nil { - fieldError = fmt.Errorf("failed to unpack field %d (%s): len %d, %w", i, fl.Spec().Description, read, err) - fields = append(fields, fl.Spec().Description) - if read == 0 { - return fields, fieldError - } + return strconv.Itoa(i), fmt.Errorf("failed to unpack field %d (%s): %w", i, fl.Spec().Description, err) } m.fieldsMap[i] = struct{}{} @@ -308,7 +299,7 @@ func (m *Message) unpack(src []byte) ([]string, error) { } } - return fields, fieldError + return "", nil } func (m *Message) MarshalJSON() ([]byte, error) { @@ -486,9 +477,6 @@ func (m *Message) Unmarshal(v interface{}) error { return errors.New("data is not a struct") } - var err error - fields := []string{} - // iterate over struct fields for i := 0; i < dataStruct.NumField(); i++ { indexTag := field.NewIndexTag(dataStruct.Type().Field(i)) @@ -513,24 +501,15 @@ func (m *Message) Unmarshal(v interface{}) error { if dataField.IsNil() && dataField.Kind() != reflect.Slice { dataField.Set(reflect.New(dataField.Type().Elem())) } - fieldErr := messageField.Unmarshal(dataField.Interface()) - if fieldErr != nil { - fields = append(fields, strconv.Itoa(indexTag.ID)) - err = fieldErr + if err := messageField.Unmarshal(dataField.Interface()); err != nil { + return fmt.Errorf("failed to get value from field %d: %w", indexTag.ID, err) } default: // Native types - fieldErr := messageField.Unmarshal(dataField) - if fieldErr != nil { - fields = append(fields, strconv.Itoa(indexTag.ID)) - err = fieldErr + if err := messageField.Unmarshal(dataField); err != nil { + return fmt.Errorf("failed to get value from field %d: %w", indexTag.ID, err) } } } - if len(fields) > 0 { - fieldStr := strings.Join(fields, ", ") - return fmt.Errorf("failed to get value from field %s: %w", fieldStr, err) - } - - return err + return nil } diff --git a/message_test.go b/message_test.go index 5c5ba4a..ad8b6d5 100644 --- a/message_test.go +++ b/message_test.go @@ -751,7 +751,7 @@ func TestPackUnpack(t *testing.T) { require.Equal(t, rawMsg, unpackError.RawMessage) }) - t.Run("Unpack data field error on final field returns partial message", func(t *testing.T) { + t.Run("Unpack data field error on field returns partial message", func(t *testing.T) { message := NewMessage(spec) rawMsg := []byte{0x30, 0x31, 0x30, 0x30, 0xf2, 0x3c, 0x24, 0x81, 0x28, 0xe0, 0x9a, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x0, 0x31, 0x36, 0x34, 0x32, 0x37, 0x36, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x37, 0x37, 0x37, 0x30, 0x30, 0x30, 0x37, 0x30, 0x31, 0x31, 0x31, 0x31, 0x38, 0x34, 0x34, 0x30, 0x30, 0x30, 0x31, 0x32, 0x33, 0x31, 0x33, 0x31, 0x38, 0x34, 0x34, 0x30, 0x37, 0x30, 0x31, 0x31, 0x39, 0x30, 0x32, 0x6, 0x43, 0x39, 0x30, 0x31, 0x30, 0x32, 0x30, 0x36, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x33, 0x37, 0x34, 0x32, 0x37, 0x36, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x3d, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x30, 0x39, 0x38, 0x37, 0x36, 0x35, 0x34, 0x33, 0x32, 0x31, 0x30, 0x30, 0x31, 0x30, 0x30, 0x30, 0x30, 0x30, 0x33, 0x32, 0x31, 0x31, 0x32, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x33, 0x34, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x54, 0x65, 0x73, 0x74, 0x20, 0x74, 0x65, 0x78, 0x74, 0x64, 0x30, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x31, 0x32, 0x33, 0x34, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x32, 0x33, 0x9a, 0x6, 0x32, 0x31, 0x30, 0x37, 0x32, 0x30, 0x9f, 0x2, 0xc, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x35, 0x30, 0x31, 0x30, 0x31, 0x37, 0x41, 0x6e, 0x6f, 0x74, 0x68, 0x65, 0x72, 0x20, 0x74, 0x65, 0x73, 0x74, 0x20, 0x74, 0x65, 0x78} @@ -761,7 +761,7 @@ func TestPackUnpack(t *testing.T) { require.Error(t, err) var unpackError *UnpackError require.ErrorAs(t, err, &unpackError) - assert.ElementsMatch(t, unpackError.Fields, []string{"Field 120"}) + assert.Equal(t, unpackError.Field, "120") s, err := message.GetString(2) require.NoError(t, err) @@ -806,130 +806,6 @@ func TestPackUnpack(t *testing.T) { assert.Empty(t, data.F120) }) - t.Run("Unpack data field error on middle field with fixed prefix returns partial message", func(t *testing.T) { - corruptSpecOut := &MessageSpec{ - Fields: map[int]field.Field{ - 0: field.NewString(&field.Spec{ - Length: 4, - Description: "Message Type Indicator", - Enc: encoding.ASCII, - Pref: prefix.ASCII.Fixed, - }), - 1: field.NewBitmap(&field.Spec{ - Description: "Bitmap", - Enc: encoding.Binary, - Pref: prefix.ASCII.Fixed, - }), - 2: field.NewString(&field.Spec{ - Length: 19, - Description: "Primary Account Number", - Enc: encoding.ASCII, - Pref: prefix.ASCII.LL, - }), - 3: field.NewNumeric(&field.Spec{ - Length: 3, - Description: "Dodgy field", - Enc: encoding.ASCII, - Pref: prefix.ASCII.Fixed, - }), - 4: field.NewString(&field.Spec{ - Length: 6, - Description: "Anything", - Enc: encoding.ASCII, - Pref: prefix.ASCII.LL, - }), - }, - } - - corruptSpecIn := &MessageSpec{ - Fields: map[int]field.Field{ - 0: field.NewString(&field.Spec{ - Length: 4, - Description: "Message Type Indicator", - Enc: encoding.ASCII, - Pref: prefix.ASCII.Fixed, - }), - 1: field.NewBitmap(&field.Spec{ - Description: "Bitmap", - Enc: encoding.Binary, - Pref: prefix.ASCII.Fixed, - }), - 2: field.NewString(&field.Spec{ - Length: 19, - Description: "Primary Account Number", - Enc: encoding.ASCII, - Pref: prefix.ASCII.LL, - }), - 3: field.NewString(&field.Spec{ - Length: 3, - Description: "Dodgy field", - Enc: encoding.ASCII, - Pref: prefix.ASCII.Fixed, - }), - 4: field.NewString(&field.Spec{ - Length: 6, - Description: "Anything", - Enc: encoding.ASCII, - Pref: prefix.ASCII.LL, - }), - }, - } - - type TestCorruptISODataIn struct { - F2 *field.String - F3 *field.String - F4 *field.String - } - - type TestCorruptISODataOut struct { - F2 *field.String - F3 *field.Numeric - F4 *field.String - } - - message := NewMessage(corruptSpecIn) - err := message.Marshal(&TestCorruptISODataIn{ - F2: field.NewStringValue("4276555555555555"), - F3: field.NewStringValue("ABC"), - F4: field.NewStringValue("123"), - }) - require.NoError(t, err) - - message.MTI("0100") - - rawMsg, err := message.Pack() - require.NoError(t, err) - - receivedMessage := NewMessage(corruptSpecOut) - - err = receivedMessage.Unpack([]byte(rawMsg)) - - require.Error(t, err) - var unpackErr *UnpackError - require.ErrorAs(t, err, &unpackErr) - require.Equal(t, []string{"Dodgy field"}, unpackErr.Fields) - - s, err := receivedMessage.GetString(2) - require.NoError(t, err) - require.Equal(t, "4276555555555555", s) - - s, err = receivedMessage.GetString(3) - require.NoError(t, err) - require.Equal(t, "0", s) - - s, err = receivedMessage.GetString(4) - require.NoError(t, err) - require.Equal(t, "123", s) - - data := &TestCorruptISODataOut{} - err = message.Unmarshal(data) - require.Error(t, err) - - assert.Equal(t, "4276555555555555", data.F2.Value()) - assert.Equal(t, int64(0), data.F3.Value()) - assert.Equal(t, "123", data.F4.Value()) - }) - // this test should check that BCD fields are packed and // unpacked correctly it's a confirmation that issue // https://github.com/moov-io/iso8583/issues/220 is fixed