ASCIIHexToBytes’ Encoded Length Is Inconsistent With Its Decoded Length

[armwich]

Overview

Version: v0.22.1

It seems like the ASCIIHexToBytes encoder's packer produces a different data length value than what the unpacker needs, which makes the unpacking process fail with the "not enough data to read" error. I have a use case to store a field as a variable hex string so that it is easy to convert between Hex strings and binary data based on the variable length part.

Example

Let's define a variable length field as the following:

myField := field.NewString(&field.Spec{
	Pref:   prefix.Binary.LL,
	Enc:    encoding.ASCIIHexToBytes,
	Length: 999,
})

Now, let's use the field to unpack binary data with a length of 3 bytes and its binary payload of 0x012345. Therefore, the raw binary input is defined as follows:

input := []byte{0x00, 0x03, 0x01, 0x23, 0x45}

Now, use the myField defined above to unpack the input data and read the parsed string out of it.

read, _ := myField.Unpack(input) // read now has a value of 5 (number of bytes read including the prefix).
readStr, _ := myField.String()   // readStr is "012345" string which means that it parses correctly.

Let's now use the myField to pack the data that has just been unpacked to see if it produces the same original data.

packed, _ := myField.Pack()  // packed turns out to be []byte{0x00, 0x06, 0x01, 0x23, 0x45}.

This instead produces a binary sequence of 0x0006012345 instead of the original 0x0003012345. The 2-byte variable length part becomes twice the original.

Potential Causes

The Unpack function of the hexToASCIIEncoder is currently defined as follows:

iso8583/encoding/hex.go

Lines 78 to 95 in 55330b0

	// Decode converts bytes into their ASCII representation.
	// Length is number of ASCII characters (two ASCII characters is one HEX digit)
	// On success, the ASCII representation bytes are returned e.g. []byte{0x5F,
	// 0x2A} would be converted to []byte("5F2A")
	func (e asciiToHexEncoder) Decode(data []byte, length int) ([]byte, int, error) {
	if length < 0 {
	return nil, 0, fmt.Errorf("length should be positive, got %d", length)
	}
	if length > len(data) {
	return nil, 0, errors.New("not enough data to read")
	}
	out := make([]byte, hex.EncodedLen(length))
	hex.Encode(out, data[:length])
	return []byte(strings.ToUpper(string(out))), length, nil
	}

According to the way the length parameter is used in the function by using its value to represent how many bytes to read from the input and creating an output array with size twice its value, this means that the length here represents input size in bytes not how many Hex characters required to represent the data. This is different from, e.g., BCD, whose length represents how many BCD digits (half-byte) in the data part not how many bytes.

Therefore, 0x0003012345 can be parsed correctly, as 0x0003 length indicates 3 bytes in the following payload part.

However, when performing packing using the same myField instance, the produced length part becomes 0x0006 instead. According to the default packer logic shown below, the length part is produced by the spec.Pref.EncodeLength(spec.Length, len(value)), which uses the length of the data before encoding. This means that the length part will always be twice the actual number of bytes because the len(value) represents the number of Hex characters, not size in bytes.

This would break the message format if this configuration is used.

iso8583/field/packer_unpacker.go

Lines 8 to 21 in 55330b0

	func (p defaultPacker) Pack(value []byte, spec *Spec) ([]byte, error) {
	// pad the value if needed
	if spec.Pad != nil {
	value = spec.Pad.Pad(value, spec.Length)
	}
	// encode the value
	encodedValue, err := spec.Enc.Encode(value)
	if err != nil {
	return nil, fmt.Errorf("failed to encode content: %w", err)
	}
	// encode the length
	lengthPrefix, err := spec.Pref.EncodeLength(spec.Length, len(value))

Questions

• Just would like to confirm whether this behavior is intentional because the ASCIIHexToBytes is also being used in multiple places, such as in BerTLVTag, and seems to work fine. It could be tricky to change the behavior since it would impact other places as well.
• Should the variable length part of the ASCIIHexToBytes format represent the actual size in bytes of the data part or number of Hex characters in the data part, just like how BCD behaves?

[alovak]

Hey! Thanks for such a detailed research.

ASCIIHexToBytes works as it should work, but because of it's nature it shouldn't be used as encoding for fields unless you use some custom field type.

It works well when we have HEX tags (or BerTLVTag) like:

Spec = &field.Spec{
		Length:      1535,
		Description: "Customer Related Data",
		Pref:        prefix.Binary.LL,
		Tag: &field.TagSpec{
			Length: 1,
			Enc:    encoding.ASCIIHexToBytes,
			Sort:   sort.StringsByHex,
		},
		Subfields: map[string]field.Field{
			"01": field.NewComposite(&field.Spec{ // DataSet ID
                        // ...

but because String, Numeric and Binary fields use value length, ASCIIHexToBytes works only for the Unpack case.

There is no reasonable way to solve that with the code for mentioned fields and ASCIIHexToBytes, we should just know that using such a combination works only one way.

We have field.Hex that should be used for cases we initially wanted to use ASCIIHexToBytes - work with HEX, but use bytes when we pack/unpack. Here is the code (https://go.dev/play/p/nniT-CtG7W4) that shows your input working both ways (unpack/pack) with field.Hex:

package main

import (
	"fmt"
	"testing"

	"github.com/moov-io/iso8583/encoding"
	"github.com/moov-io/iso8583/field"
	"github.com/moov-io/iso8583/prefix"
)

func TestTest(t *testing.T) {
	myField := field.NewString(&field.Spec{
		Pref:   prefix.Binary.LL,
		Enc:    encoding.ASCIIHexToBytes,
		Length: 999,
	})

	input := []byte{0x00, 0x03, 0x01, 0x23, 0x45}

	myField.Unpack(input)          // read now has a value of 5 (number of bytes read including the prefix).
	readStr, _ := myField.String() // readStr is "012345" string which means that it parses correctly.

	packed, _ := myField.Pack() // packed turns out to be []byte{0x00, 0x06, 0x01, 0x23, 0x45}.

	fmt.Printf("input: %X\n", input)
	fmt.Printf("read string: %s\n", readStr)
	fmt.Printf("packed: %X\n", packed)
	

	// do the same for Hex field
	myHexField := field.NewHex(&field.Spec{
		Pref:   prefix.Binary.LL,
		Enc:    encoding.Binary, // We should use Binary encoding here
		Length: 999,
	})

	myHexField.Unpack(input)
	readStr, _ = myHexField.String() // readStr is "012345" string which means that it parses correctly.
	fmt.Printf("read string: %s\n", readStr)

	packed, _ = myHexField.Pack() // packed matches the input []byte{0x00, 0x03, 0x01, 0x23, 0x45}.
	fmt.Printf("packed: %X\n", packed)

}

P.S. I tried to fix some places to avoid issues like this here #329 and here #328.

[armwich]

Hi, thank you very much for the great details! Those answer all my questions. I'll use something like a custom encoder to handle my use case instead of directly using the ASCIIHexToBytes encoder.