diff --git a/errors.go b/errors.go deleted file mode 100644 index b0643eb8..00000000 --- a/errors.go +++ /dev/null @@ -1,29 +0,0 @@ -package iso8583 - -// UnpackError returns error with possibility to access RawMessage when -// connection failed to unpack message -type UnpackError struct { - Err error - FieldID string - RawMessage []byte -} - -func (e *UnpackError) Error() string { - return e.Err.Error() -} - -func (e *UnpackError) Unwrap() error { - return e.Err -} - -type PackError struct { - Err error -} - -func (e *PackError) Error() string { - return e.Err.Error() -} - -func (e *PackError) Unwrap() error { - return e.Err -} diff --git a/errors/errors.go b/errors/errors.go new file mode 100644 index 00000000..ec2f9060 --- /dev/null +++ b/errors/errors.go @@ -0,0 +1,50 @@ +package errors + +import ( + "errors" +) + +// UnpackError returns an error with the possibility to access the RawMessage when +// the connection failed to unpack the message +type UnpackError struct { + Err error + FieldID string + RawMessage []byte +} + +func (e *UnpackError) Error() string { + return e.Err.Error() +} + +func (e *UnpackError) Unwrap() error { + return e.Err +} + +// FieldIDs returns the list of field and subfield IDs (if any) that errored from outermost inwards +func (e *UnpackError) FieldIDs() []string { + fieldIDs := []string{e.FieldID} + err := e.Err + var unpackError *UnpackError + for { + if errors.As(err, &unpackError) { + fieldIDs = append(fieldIDs, unpackError.FieldID) + err = unpackError.Unwrap() + } else { + break + } + } + + return fieldIDs +} + +type PackError struct { + Err error +} + +func (e *PackError) Error() string { + return e.Err.Error() +} + +func (e *PackError) Unwrap() error { + return e.Err +} diff --git a/field/composite.go b/field/composite.go index 0db1399f..b119b120 100644 --- a/field/composite.go +++ b/field/composite.go @@ -10,6 +10,7 @@ import ( "sync" "github.com/moov-io/iso8583/encoding" + iso8583errors "github.com/moov-io/iso8583/errors" "github.com/moov-io/iso8583/prefix" "github.com/moov-io/iso8583/sort" "github.com/moov-io/iso8583/utils" @@ -312,7 +313,7 @@ func (f *Composite) Unpack(data []byte) (int, error) { // 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], isVariableLength) + read, err := f.wrapErrorUnpack(data[offset:offset+dataLen], isVariableLength) if err != nil { return 0, err } @@ -331,7 +332,7 @@ func (f *Composite) SetBytes(data []byte) error { f.mu.Lock() defer f.mu.Unlock() - _, err := f.unpack(data, false) + _, err := f.wrapErrorUnpack(data, false) return err } @@ -515,7 +516,22 @@ func (f *Composite) packByTag() ([]byte, error) { return packed, nil } -func (f *Composite) unpack(data []byte, isVariableLength bool) (int, error) { +// wrapErrorUnpack calls the core unpacking logic and wraps any +// errors in a *UnpackError. It assumes that the mutex is already +// locked by the caller. +func (f *Composite) wrapErrorUnpack(src []byte, isVariableLength bool) (int, error) { + offset, tagID, err := f.unpack(src, isVariableLength) + if err != nil { + return offset, &iso8583errors.UnpackError{ + Err: err, + FieldID: tagID, + RawMessage: src, + } + } + return offset, nil +} + +func (f *Composite) unpack(data []byte, isVariableLength bool) (int, string, error) { if f.bitmap() != nil { return f.unpackSubfieldsByBitmap(data) } @@ -525,7 +541,7 @@ func (f *Composite) unpack(data []byte, isVariableLength bool) (int, error) { return f.unpackSubfields(data, isVariableLength) } -func (f *Composite) unpackSubfields(data []byte, isVariableLength bool) (int, error) { +func (f *Composite) unpackSubfields(data []byte, isVariableLength bool) (int, string, error) { offset := 0 for _, tag := range f.orderedSpecFieldTags { field, ok := f.subfields[tag] @@ -535,7 +551,7 @@ func (f *Composite) unpackSubfields(data []byte, isVariableLength bool) (int, er read, err := field.Unpack(data[offset:]) if err != nil { - return 0, fmt.Errorf("failed to unpack subfield %v: %w", tag, err) + return 0, tag, fmt.Errorf("failed to unpack subfield %v: %w", tag, err) } f.setSubfields[tag] = struct{}{} @@ -547,10 +563,10 @@ func (f *Composite) unpackSubfields(data []byte, isVariableLength bool) (int, er } } - return offset, nil + return offset, "", nil } -func (f *Composite) unpackSubfieldsByBitmap(data []byte) (int, error) { +func (f *Composite) unpackSubfieldsByBitmap(data []byte) (int, string, error) { var off int // Reset fields that were set. @@ -560,7 +576,7 @@ func (f *Composite) unpackSubfieldsByBitmap(data []byte) (int, error) { read, err := f.bitmap().Unpack(data[off:]) if err != nil { - return 0, fmt.Errorf("failed to unpack bitmap: %w", err) + return 0, "", fmt.Errorf("failed to unpack bitmap: %w", err) } off += read @@ -571,12 +587,12 @@ func (f *Composite) unpackSubfieldsByBitmap(data []byte) (int, error) { fl, ok := f.subfields[iStr] if !ok { - return 0, fmt.Errorf("failed to unpack subfield %s: no specification found", iStr) + return 0, iStr, fmt.Errorf("failed to unpack subfield %s: no specification found", iStr) } read, err = fl.Unpack(data[off:]) if err != nil { - return 0, fmt.Errorf("failed to unpack subfield %s (%s): %w", iStr, fl.Spec().Description, err) + return 0, iStr, fmt.Errorf("failed to unpack subfield %s (%s): %w", iStr, fl.Spec().Description, err) } f.setSubfields[iStr] = struct{}{} @@ -585,7 +601,7 @@ func (f *Composite) unpackSubfieldsByBitmap(data []byte) (int, error) { } } - return off, nil + return off, "", nil } const ( @@ -596,12 +612,12 @@ const ( maxLenOfUnknownTag = math.MaxInt ) -func (f *Composite) unpackSubfieldsByTag(data []byte) (int, error) { +func (f *Composite) unpackSubfieldsByTag(data []byte) (int, string, error) { offset := 0 for offset < len(data) { tagBytes, read, err := f.spec.Tag.Enc.Decode(data[offset:], f.spec.Tag.Length) if err != nil { - return 0, fmt.Errorf("failed to unpack subfield Tag: %w", err) + return 0, "", fmt.Errorf("failed to unpack subfield Tag: %w", err) } offset += read @@ -623,15 +639,15 @@ func (f *Composite) unpackSubfieldsByTag(data []byte) (int, error) { maxLen = maxLenOfUnknownTag } - fieldLength, readed, err := pref.DecodeLength(maxLen, data[offset:]) + fieldLength, read, err := pref.DecodeLength(maxLen, data[offset:]) if err != nil { - return 0, err + return 0, "", err } - offset += fieldLength + readed + offset += fieldLength + read continue } - return 0, fmt.Errorf("failed to unpack subfield %v: field not defined in Spec", tag) + return 0, tag, fmt.Errorf("failed to unpack subfield %v: field not defined in Spec", tag) } field, ok := f.subfields[tag] @@ -641,14 +657,14 @@ func (f *Composite) unpackSubfieldsByTag(data []byte) (int, error) { read, err = field.Unpack(data[offset:]) if err != nil { - return 0, fmt.Errorf("failed to unpack subfield %v: %w", tag, err) + return 0, tag, fmt.Errorf("failed to unpack subfield %v: %w", tag, err) } f.setSubfields[tag] = struct{}{} offset += read } - return offset, nil + return offset, "", nil } func (f *Composite) skipUnknownTLVTags() bool { diff --git a/field/composite_test.go b/field/composite_test.go index adef77fb..7651b88e 100644 --- a/field/composite_test.go +++ b/field/composite_test.go @@ -8,9 +8,11 @@ import ( "testing" "github.com/moov-io/iso8583/encoding" + iso8583errors "github.com/moov-io/iso8583/errors" "github.com/moov-io/iso8583/padding" "github.com/moov-io/iso8583/prefix" "github.com/moov-io/iso8583/sort" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -799,6 +801,10 @@ func TestCompositePacking(t *testing.T) { read, err := composite.Unpack([]byte("ABCDEF")) require.Equal(t, 0, read) require.Error(t, err) + var unpackError *iso8583errors.UnpackError + require.ErrorAs(t, err, &unpackError) + assert.Equal(t, "3", unpackError.FieldID) + assert.Equal(t, []string{"3"}, unpackError.FieldIDs()) require.EqualError(t, err, "failed to unpack subfield 3: failed to set bytes: failed to convert into number") require.ErrorIs(t, err, strconv.ErrSyntax) }) diff --git a/message.go b/message.go index d9411ab0..1a6640a7 100644 --- a/message.go +++ b/message.go @@ -9,6 +9,7 @@ import ( "strconv" "sync" + iso8583errors "github.com/moov-io/iso8583/errors" "github.com/moov-io/iso8583/field" "github.com/moov-io/iso8583/utils" ) @@ -175,7 +176,7 @@ func (m *Message) Pack() ([]byte, error) { func (m *Message) wrapErrorPack() ([]byte, error) { data, err := m.pack() if err != nil { - return nil, &PackError{Err: err} + return nil, &iso8583errors.PackError{Err: err} } return data, nil @@ -238,7 +239,7 @@ func (m *Message) Unpack(src []byte) error { // locked by the caller. func (m *Message) wrapErrorUnpack(src []byte) error { if fieldID, err := m.unpack(src); err != nil { - return &UnpackError{ + return &iso8583errors.UnpackError{ Err: err, FieldID: fieldID, RawMessage: src, diff --git a/message_test.go b/message_test.go index 9e7170ba..09fd1851 100644 --- a/message_test.go +++ b/message_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/moov-io/iso8583/encoding" + iso8583errors "github.com/moov-io/iso8583/errors" "github.com/moov-io/iso8583/field" "github.com/moov-io/iso8583/padding" "github.com/moov-io/iso8583/prefix" @@ -722,7 +723,7 @@ func TestPackUnpack(t *testing.T) { _, err := message.Pack() require.Error(t, err) - var packErr *PackError + var packErr *iso8583errors.PackError require.ErrorAs(t, err, &packErr) }) @@ -733,7 +734,7 @@ func TestPackUnpack(t *testing.T) { require.Error(t, err) - var unpackError *UnpackError + var unpackError *iso8583errors.UnpackError require.ErrorAs(t, err, &unpackError) }) @@ -746,7 +747,7 @@ func TestPackUnpack(t *testing.T) { require.Error(t, err) - var unpackError *UnpackError + var unpackError *iso8583errors.UnpackError require.ErrorAs(t, err, &unpackError) require.Equal(t, rawMsg, unpackError.RawMessage) }) @@ -760,7 +761,7 @@ func TestPackUnpack(t *testing.T) { err := message.Unpack([]byte(rawMsg)) require.Error(t, err) - var unpackError *UnpackError + var unpackError *iso8583errors.UnpackError require.ErrorAs(t, err, &unpackError) assert.Equal(t, unpackError.FieldID, "120") @@ -807,6 +808,221 @@ func TestPackUnpack(t *testing.T) { assert.Empty(t, data.F120) }) + t.Run("Unpack data field error on subfield returns both numbers and partial message", func(t *testing.T) { + type TestISOF3MixedData struct { + F1 *field.String + F2 *field.Numeric + F3 *field.String + } + + type TestISOShortData struct { + F2 *field.String + F3 *TestISOF3MixedData + } + + shortSpec := &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.NewComposite(&field.Spec{ + Length: 6, + Description: "Processing Code", + Pref: prefix.ASCII.Fixed, + Tag: &field.TagSpec{ + Sort: sort.StringsByInt, + }, + Subfields: map[string]field.Field{ + "1": field.NewString(&field.Spec{ + Length: 2, + Description: "Transaction Type", + Enc: encoding.ASCII, + Pref: prefix.ASCII.Fixed, + }), + "2": field.NewNumeric(&field.Spec{ + Length: 2, + Description: "From Account", + Enc: encoding.ASCII, + Pref: prefix.ASCII.Fixed, + Pad: padding.Left('0'), + }), + "3": field.NewString(&field.Spec{ + Length: 2, + Description: "To Account", + Enc: encoding.ASCII, + Pref: prefix.ASCII.Fixed, + }), + }, + }), + }, + } + message := NewMessage(shortSpec) + message.MTI("0100") + + // Field F3SF2 has a letter in, which will make it fail + rawMsg := []byte{0x30, 0x31, 0x30, 0x30, // MTI F0 + 0x60, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // Bitmap F1 + 0x31, 0x36, // Tag for F2 + 0x34, 0x32, 0x37, 0x36, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, //F2 + 0x30, 0x30, 0x30, 0x51, 0x30, 0x30, //F3 + } + + err := message.Unpack([]byte(rawMsg)) + + require.Error(t, err) + var unpackError *iso8583errors.UnpackError + require.ErrorAs(t, err, &unpackError) + assert.Equal(t, "3", unpackError.FieldID) + assert.Equal(t, []string{"3", "2"}, unpackError.FieldIDs()) + + 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, "00", s) + + data := &TestISOShortData{} + require.NoError(t, message.Unmarshal(data)) + + assert.Equal(t, "4276555555555555", data.F2.Value()) + }) + + t.Run("Unpack data field error on subfield in nested spec returns both numbers and partial message", func(t *testing.T) { + type TestDateAndTimeData struct { + F1 *field.String + F2 *field.Numeric + } + + type TestISOF3MixedData struct { + F1 *field.String + F2 *TestDateAndTimeData + F3 *field.String + } + + type TestISOShortData struct { + F2 *field.String + F3 *TestISOF3MixedData + } + + shortSpec := &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.NewComposite(&field.Spec{ + Length: 14, + Description: "Processing Code", + Pref: prefix.ASCII.LL, + Tag: &field.TagSpec{ + Sort: sort.StringsByInt, + }, + Subfields: map[string]field.Field{ + "1": field.NewString(&field.Spec{ + Length: 2, + Description: "Transaction Type", + Enc: encoding.ASCII, + Pref: prefix.ASCII.Fixed, + }), + "2": field.NewComposite(&field.Spec{ + Length: 10, + Description: "Authorization System Advice Date and Time", + Pref: prefix.ASCII.Fixed, + Tag: &field.TagSpec{ + Sort: sort.StringsByInt, + }, + Subfields: map[string]field.Field{ + "1": field.NewString(&field.Spec{ + Length: 4, + Description: "Date", + Enc: encoding.ASCII, + Pref: prefix.ASCII.Fixed, + }), + "2": field.NewNumeric(&field.Spec{ + Length: 6, + Description: "Time", + Enc: encoding.ASCII, + Pref: prefix.ASCII.Fixed, + }), + }, + }), + "3": field.NewString(&field.Spec{ + Length: 2, + Description: "To Account", + Enc: encoding.ASCII, + Pref: prefix.ASCII.Fixed, + }), + }, + }), + }, + } + message := NewMessage(shortSpec) + message.MTI("0100") + + // Field F3SF2SF2 is a numeric type but has a letter in, which will cause an unpack error + rawMsg := []byte{0x30, 0x31, 0x30, 0x30, // MTI F0 + 0x60, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // Bitmap F1 + 0x31, 0x36, // Tag for F2 + 0x34, 0x32, 0x37, 0x36, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, 0x35, //F2 + 0x31, 0x34, // Tag for F3 + 0x30, 0x30, // F3 SF1 + 0x31, 0x31, 0x31, 0x31, //F3 SF2 Subfield 1 + 0x31, 0x32, 0x33, 0x34, 0x35, 0x51, //F3 SF2 Subfield 2 + 0x30, 0x30, // F3 SF3 + } + + err := message.Unpack([]byte(rawMsg)) + + require.Error(t, err) + var unpackError *iso8583errors.UnpackError + require.ErrorAs(t, err, &unpackError) + assert.Equal(t, "3", unpackError.FieldID) + assert.Equal(t, []string{"3", "2", "2"}, unpackError.FieldIDs()) + + 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, "00", s) + + data := &TestISOShortData{} + require.NoError(t, message.Unmarshal(data)) + + assert.Equal(t, "4276555555555555", data.F2.Value()) + require.Nil(t, data.F3) + }) + // 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