Today, Ivan sends us an acknowledgment. This is its code, which is made to process ISO 8583 messages. As we will see from some of the later comments, John knows this is bad.
The ISO 8583 format is mostly used in financial transaction processing, often for communication with ATMs, but it’s likely to appear somewhere in every transaction you do that isn’t pure cash.
One of the things this format can support is bitmaps – not the image format, but the “actual tags to integer” format. John wrote his own version of this, in C#. The lecture is long, so I will focus only on the most important.
private readonly bool[] bits;
Look, we’re not off to a great start. This isn’t an absolute bug, but if you’re working on a data structure that’s meant to be manipulated via bitwise operations, just rely on it. And yes, if endianness is an issue, you’ll have to think a little harder – but you have to think about it anyway. Use clear method names and documentation to make it readable.
In this developer’s defense, the maximum bitmap size is 128 bits, which has no native integral type in C#, but a 64-bit pair would be easier to understand, at least for me. Maybe I just got infected with low level software worms. Okay, let’s use an array.
Now, one thing that is important, is that we use this bitmap for representation more things.
public bool IsExtendedBitmap
get
return this.IsFieldSet(1);
Notice how the 1st bit is in this bitmap IsExtendedBitmap flag. This controls the length of the total bitmap.
What, by the way, do they use IsFieldSet because zero-based indexes are too difficult:
public bool IsFieldSet(int field)
return this.bits[field - 1];
But things get worse.
public void SetField(int field, bool on)
this.bits[field - 1] = on;
this.bits[0] = false;
for (var i = 64; i <= 127; i++)
if (this.bits[i])
this.bits[0] = true;
break;
I’ve included the comments here because I want to highlight how useless they are. The first line makes sense. Then we set the first bit to false. What, um, was IsExtendedBitmap flag. Why? I don’t know Then we iterate over the back half of the bitmap and if there is something true there, we return that first bit to true.
Which, by writing the last paragraph, he figured out what it does: it automatically detects if you’re using high-order bits and sets it IsExtendedBitmap as needed. I’m not sure this is actually the correct behavior – what happens if I want to explicitly set the high-order bit to 0? – but I haven’t read the spec, so we’ll leave it alone.
public virtual byte[] ToMsg()
var lengthOfBitmap = this.IsExtendedBitmap ? 16 : 8;
var data = new byte[lengthOfBitmap];
for (var i = 0; i < lengthOfBitmap; i++)
for (var j = 0; j < 8; j++)
if (this.bits[i * 8 + j])
(128 / (int)Math.Pow(2, j)));
if (this.formatter is BinaryFormatter)
return data;
IFormatter binaryFormatter = new BinaryFormatter();
var bitmapString = binaryFormatter.GetString(data);
return this.formatter.GetBytes(bitmapString);
Here is our serialization method. Note that the length of the bitmap here is either 8 or 16, whereas previously we checked all bits from 64 and up to see if it was extended. At first it looked wrong, but then I realized- data is byte[]– so 16 bytes is really 128 bits.
This gives them the challenging problem of dealing with individual bits within this data structure, and they obviously don’t know how bitwise operations work, so we get a nice Math.Pow(2, j) in.
Ugly, for sure. Vaguely, definitely. Which gets worse when we start unpacking.
public int Unpack(byte[] msg, int offset)
var lengthOfBitmap = this.formatter.GetPackedLength(16);
if (this.formatter is BinaryFormatter)
if (msg[offset] >= 128)
lengthOfBitmap += 8;
else
if (msg[offset] >= 0x38)
lengthOfBitmap += 16;
var bitmapData = new byte[lengthOfBitmap];
Array.Copy(msg, offset, bitmapData, 0, lengthOfBitmap);
if (!(this.formatter is BinaryFormatter))
IFormatter binaryFormatter = new BinaryFormatter();
var value = this.formatter.GetString(bitmapData);
bitmapData = binaryFormatter.GetBytes(value);
for (var j = 0; j < 8; j++)
this.bits[i * 8 + j] = (bitmapData[i] & (128 / (int)Math.Pow(2, j))) > 0;
return offset + lengthOfBitmap;
This is where we get our real highlights: the comments. “… but it works… I think…”. “Good luck figuring this out. There are dragons down there.”
Now, John wrote this code some time ago. And what I realize, when I read this code, is that John was probably a bit green and didn’t fully understand the problem in front of him or the tools available to him to solve it. Furthermore, this was John’s independent project, working to solve a very specific problem – so even though the code has issues, I wouldn’t blame John too much for it.
Which, like many other confessional daily code samples, I’m sharing because I think it’s an interesting learning experience. It’s less “WTF!” and more, “Oh man, I see things have gone wrong for you.” We all make mistakes and we all write terrible code from time to time. Kudos to John for sharing this error.
[Advertisement]
Otter – Secure your servers automatically without the need to log into the command line. Get started today!