From 508a839b39a3fb290a46822e366d145cc6fdefe3 Mon Sep 17 00:00:00 2001 From: eileen Date: Mon, 20 Nov 2023 17:07:18 +0000 Subject: [PATCH] Return partial message when unpack errors --- errors.go | 4 +- field/numeric.go | 4 +- field/string.go | 4 +- message.go | 48 +++++++++---- message_test.go | 179 +++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 220 insertions(+), 19 deletions(-) diff --git a/errors.go b/errors.go index 3a63def..6aea3a3 100644 --- a/errors.go +++ b/errors.go @@ -4,6 +4,7 @@ package iso8583 // connection failed to unpack message type UnpackError struct { Err error + Fields []string RawMessage []byte } @@ -16,7 +17,8 @@ func (e *UnpackError) Unwrap() error { } type PackError struct { - Err error + Fields []string + Err error } func (e *PackError) Error() string { diff --git a/field/numeric.go b/field/numeric.go index 1c0f854..a3c485c 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 0, fmt.Errorf("failed to decode content: %w", err) + return dataLen, 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 0, fmt.Errorf("failed to set bytes: %w", err) + return dataLen, fmt.Errorf("failed to set bytes: %w", err) } return read + prefBytes, nil diff --git a/field/string.go b/field/string.go index c8ce11c..0819460 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 0, fmt.Errorf("failed to decode content: %w", err) + return dataLen, 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 0, fmt.Errorf("failed to set bytes: %w", err) + return dataLen, fmt.Errorf("failed to set bytes: %w", err) } return read + prefBytes, nil diff --git a/message.go b/message.go index d650342..6ec996a 100644 --- a/message.go +++ b/message.go @@ -7,6 +7,7 @@ import ( "reflect" "sort" "strconv" + "strings" "sync" "github.com/moov-io/iso8583/field" @@ -237,9 +238,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 err := m.unpack(src); err != nil { + if fields, err := m.unpack(src); err != nil { return &UnpackError{ Err: err, + Fields: fields, RawMessage: src, } } @@ -249,8 +251,10 @@ 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) 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{}{} @@ -260,7 +264,7 @@ func (m *Message) unpack(src []byte) error { read, err := m.fields[mtiIdx].Unpack(src) if err != nil { - return fmt.Errorf("failed to unpack MTI: %w", err) + return nil, fmt.Errorf("failed to unpack MTI: %w", err) } m.fieldsMap[mtiIdx] = struct{}{} @@ -270,7 +274,7 @@ func (m *Message) unpack(src []byte) error { // unpack Bitmap read, err = m.fields[bitmapIdx].Unpack(src[off:]) if err != nil { - return fmt.Errorf("failed to unpack bitmap: %w", err) + return nil, fmt.Errorf("failed to unpack bitmap: %w", err) } off += read @@ -284,12 +288,18 @@ func (m *Message) unpack(src []byte) error { if m.bitmap().IsSet(i) { fl, ok := m.fields[i] if !ok { - return fmt.Errorf("failed to unpack field %d: no specification found", i) + fieldError = fmt.Errorf("failed to unpack field %d: no specification found", i) + fields = append(fields, strconv.Itoa(i)) + return fields, fieldError } read, err = fl.Unpack(src[off:]) if err != nil { - return fmt.Errorf("failed to unpack field %d (%s): %w", i, fl.Spec().Description, err) + 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 + } } m.fieldsMap[i] = struct{}{} @@ -298,7 +308,7 @@ func (m *Message) unpack(src []byte) error { } } - return nil + return fields, fieldError } func (m *Message) MarshalJSON() ([]byte, error) { @@ -476,6 +486,9 @@ 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)) @@ -500,17 +513,24 @@ func (m *Message) Unmarshal(v interface{}) error { if dataField.IsNil() && dataField.Kind() != reflect.Slice { dataField.Set(reflect.New(dataField.Type().Elem())) } - err := messageField.Unmarshal(dataField.Interface()) - if err != nil { - return fmt.Errorf("failed to get value from field %d: %w", indexTag.ID, err) + fieldErr := messageField.Unmarshal(dataField.Interface()) + if fieldErr != nil { + fields = append(fields, strconv.Itoa(indexTag.ID)) + err = fieldErr } default: // Native types - err := messageField.Unmarshal(dataField) - if err != nil { - return fmt.Errorf("failed to get value from field %d: %w", indexTag.ID, err) + fieldErr := messageField.Unmarshal(dataField) + if fieldErr != nil { + fields = append(fields, strconv.Itoa(indexTag.ID)) + err = fieldErr } } } - return nil + if len(fields) > 0 { + fieldStr := strings.Join(fields, ", ") + return fmt.Errorf("failed to get value from field %s: %w", fieldStr, err) + } + + return err } diff --git a/message_test.go b/message_test.go index 912c726..5c5ba4a 100644 --- a/message_test.go +++ b/message_test.go @@ -751,6 +751,185 @@ 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) { + 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} + + err := message.Unpack([]byte(rawMsg)) + + require.Error(t, err) + var unpackError *UnpackError + require.ErrorAs(t, err, &unpackError) + assert.ElementsMatch(t, unpackError.Fields, []string{"Field 120"}) + + s, err := message.GetString(2) + require.NoError(t, err) + require.Equal(t, "4276555555555555", s) + + s, err = message.GetString(3) + require.NoError(t, err) + require.Equal(t, "000000", s) + + s, err = message.GetString(4) + require.NoError(t, err) + require.Equal(t, "77700", s) + + data := &TestISOData{} + require.NoError(t, message.Unmarshal(data)) + + assert.Equal(t, "4276555555555555", data.F2.Value()) + assert.Equal(t, "00", data.F3.F1.Value()) + assert.Equal(t, "00", data.F3.F2.Value()) + assert.Equal(t, "00", data.F3.F3.Value()) + assert.Equal(t, int64(77700), data.F4.Value()) + assert.Equal(t, int64(701111844), data.F7.Value()) + assert.Equal(t, int64(123), data.F11.Value()) + assert.Equal(t, int64(131844), data.F12.Value()) + assert.Equal(t, int64(701), data.F13.Value()) + assert.Equal(t, int64(1902), data.F14.Value()) + assert.Equal(t, int64(643), data.F19.Value()) + assert.Equal(t, int64(901), data.F22.Value()) + assert.Equal(t, int64(2), data.F25.Value()) + assert.Equal(t, int64(123456), data.F32.Value()) + assert.Equal(t, "4276555555555555=12345678901234567890", data.F35.Value()) + assert.Equal(t, "987654321001", data.F37.Value()) + assert.Equal(t, "00000321", data.F41.Value()) + assert.Equal(t, "120000000000034", data.F42.Value()) + assert.Equal(t, "Test text", data.F43.Value()) + assert.Equal(t, int64(643), data.F49.Value()) + assert.Nil(t, data.F50) + assert.Equal(t, string([]byte{1, 2, 3, 4, 5, 6, 7, 8}), data.F52.Value()) + assert.Equal(t, int64(1234000000000000), data.F53.Value()) + assert.Equal(t, "210720", data.F55.F9A.Value()) + assert.Equal(t, "000000000501", data.F55.F9F02.Value()) + 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