knovichikhin / pyiso8583

ISO8583 protocol parser that creates a regular Python dictionary describing ISO8583 data
MIT License
51 stars 16 forks source link

Signed BCD values #22

Closed michael-robinson-kb closed 1 year ago

michael-robinson-kb commented 1 year ago

Does pyiso8583 support signed BCD values? eg

0xD1 0x23

being decoded as -123?

knovichikhin commented 1 year ago

It's not supported, But you can still send it. And receive it in the same way. But it will not translate it to the regular sign. It's possible to add support, but I don't have time for this in the next few months.

import iso8583
from iso8583.specs import default_ascii as spec
# Test binary field
spec["112"] = {
    "data_enc": "b",
    "len_enc": "bcd",
    "len_type": 2, # LLVAR
    "len_count": "nibbles",
    "left_pad": "0",
    "max_len": 4
}
message = {"t": "0200", "112": "D123"}
raw_message, message_encoded = iso8583.encode(message, spec)
print(raw_message)
# bytearray(b'020080000000000000000000000000010000\x00\x04\xd1#')
print([bin(byte) for byte in message_encoded["112"]["data"]])
# ['0b11010001', '0b100011']

message = {"t": "0200", "112": "D12"}
raw_message, message_encoded = iso8583.encode(message, spec)
print(raw_message)
# bytearray(b'020080000000000000000000000000010000\x00\x03\r\x12')
print([bin(byte) for byte in message_encoded["112"]["data"]])
# ['0b1101', '0b10010']

Also FYI, this library does not encode/decode integers, only strings. For example, in this case if someone sends binary data D123D321 in a BCD field, the decoded string will be -123-321. It's up to user code to unpack it into two integers.

michael-robinson-kb commented 1 year ago

That's awesome, thanks. I can work with that, and possibly even find time to contribute a patch back to support.

knovichikhin commented 1 year ago

I looked into it a bit. https://en.wikipedia.org/wiki/Binary-coded_decimal#Packed_BCD covers signs in BCD.

In your experience, do you see any problem if this library decodes according to that table? I.e. A, C, E, F will be converted to +, and B, D will be converted to -.

In your experience, do you see any problem if this library encodes + as C and - as D? Does anybody ever expects other values or unsigned? Does it need to be configurable? For example, is there a case where it's correct to send F123 (unsigned) instead of C123 (positive)?

A bit about current implementation. This library supports BCD encoding/decoding as a subset of hex to binary (and back) conversion. That works because 0-9 are encoded the same in BCD and hex. This library can have a step to convert A-F to and from BCD signs, but this step is purely convenience and anybody can do it now.

For example, let's take incoming binary data A123B456C789D123F123 (or in binary b'\xa1\x23\xb4\x56\xc7\x89\xd1\x23\xf1\x23'):

# Currently this is decoded as:
b'\xa1\x23\xb4\x56\xc7\x89\xd1\x23\xf1\x23'.hex().upper()
# 'A123B456C789D123F123'

# Specifically for BCD we can convert this hex decoded value:
b'\xa1\x23\xb4\x56\xc7\x89\xd1\x23\xf1\x23'.hex().translate({97: '+', 98: '-', 99: '+', 100: '-', 102: '+'})
# '+123-456+789-123+123'

So it's more of a convenience thing than anything you can't do today.

knovichikhin commented 1 year ago

So I thought about this some more. And this is going to be a no fix for the following reasons:

  1. There is ambiguity about encoding the sign. Different systems expect different things. Of course, it can be configurable, but then it will complicate things more while not adding much value (see point 2)
  2. What you are trying to do can already be achieve with a bit of postprocessing that's very trivial.

This is an example of what you can do to achieve +/- sign.

import iso8583
from iso8583.specs import default_ascii as spec

translation_table = str.maketrans({"A": "+", "B": "-", "C": "+", "D": "-", "F": "+"})

# Make field 28 a fixed binary field of 5 bytes - just for demonstration
spec["28"] = {
    "data_enc": "b",
    "len_enc": "ascii",
    "len_type": 0,
    "max_len": 5,
    "desc": "Amount, Transaction Fee",
}

# Suppose we receive this message
incoming_message = b'02000000001000000000\xc1#Eg\x89'

# Decode the message
decoded_message, broken_down_encoded_message = iso8583.decode(incoming_message, spec)
decoded_message["28"].translate(translation_table)
# '+123456789'