prefix.BCD.Fixed doesn't calculate the encoding length correctly

[ddvk]

When using BCD.Fixed, my expectation was that the length is in Nibbles i.e. number of digits.

However, encoding checks and uses the length in bytes, but the Decoding uses the length in digits.

There is no error during packing, and the other side cannot correctly decode the message as the field is larger than the spec and further fields are clobbered.

Example

package main

import (
    "encoding/hex"
    "testing"

    "github.com/moov-io/iso8583"
    "github.com/moov-io/iso8583/encoding"
    "github.com/moov-io/iso8583/field"
    "github.com/moov-io/iso8583/padding"
    "github.com/moov-io/iso8583/prefix"
    "github.com/stretchr/testify/require"
)

var spec = &iso8583.MessageSpec{
    Fields: map[int]field.Field{
        0: field.NewNumeric(&field.Spec{
            Length:      4,
            Description: "Message Type Indicator",
            Enc:         encoding.BCD,
            Pref:        prefix.BCD.Fixed,
        }),
        1: field.NewBitmap(&field.Spec{
            Description: "Bitmap",
            Enc:         encoding.Binary,
            Pref:        prefix.Binary.Fixed,
        }),
        2: field.NewNumeric(&field.Spec{
            Length:      2,
            Description: "SomeFixedField",
            Enc:         encoding.BCD,
            Pref:        prefix.BCD.Fixed,
            Pad:         padding.Left('0'),
        }),
        3: field.NewNumeric(&field.Spec{
            Length:      3,
            Description: "SomeVarField",
            Enc:         encoding.BCD,
            Pref:        prefix.BCD.LLLL,
        }),
    },
}

func TestFixed(t *testing.T) {
    msg := iso8583.NewMessage(spec)
    msg.MTI("1234")
    field2 := "1234"
    msg.Field(2, field2)
    out, err := msg.Pack()
    require.NoError(t, err)
    t.Log(hex.EncodeToString(out))
    in := iso8583.NewMessage(spec)
    err = in.Unpack(out)
    require.NoError(t, err)
    result, _ := in.GetField(2).String()
    require.Equal(t, field2, result)
}

func TestVariable(t *testing.T) {
    msg := iso8583.NewMessage(spec)
    msg.MTI("1234")
    field3 := "456"
    msg.Field(3, field3)
    out, err := msg.Pack()
    require.NoError(t, err)
    t.Log(hex.EncodeToString(out))
    in := iso8583.NewMessage(spec)
    err = in.Unpack(out)
    require.NoError(t, err)
    result, _ := in.GetField(3).String()
    require.Equal(t, field3, result)
}

[alovak]

Hey! Thanks for reporting the issue! I’ll take a look.

[alovak]

@ddvk Sorry for such a long delay with the issue review. I see one problem with the example you provided. For field 2 length is Length: 2, but you set value of the length 4 (1234). If you set Length: 4 it will work as expected.

Length - is not about the final encoded length. As it says here https://github.com/moov-io/iso8583/blob/master/field/spec.go#L43:

    // Length defines the maximum length of field (bytes, characters or
    // digits), for both fixed and variable lengths.

Using 34 as field value works for me when length is 2. 1234 works for me when length is 4.

[alovak]

@ddvk while Length in the documentation says that it's about field length, in the prefixer code I see we are using the length of the encoded data. I need to review it one more time.

[ddvk]

I had 2 issues. in the example, I would expect the field length to apply to the field data. In variable length encoding, it get's truncated. updated the code the example code