[SNMP4J] Apparent Bug in Counter64

[Jim Doble]

The getBERLength method in the Counter64 class appears to be incorrect. For
example, if I run a test program with the following code:

    public static void main(String[] args) throws Exception {
      System.err.println(new Counter64(127).getBERLength());
      System.err.println(new Counter64(128).getBERLength());
    }

I get the following output:

    3
    7

The first value is correct, because the proper encoding is:

    46 01 7F

but the second result is incorrect, because the proper encoding is:

    46 02 00 80

The existing algorithm for the getBERLength is:

    if (((value >> 56) & 0x80) > 0) {
      return 11; // 8+3;
    }
    for (int i=7; i>=0; i--) {
    	if ((value & (0x80 << (i*8))) != 0) {
        return i + 3;
      }
    }
    return 3;

The first problem I found with this routine is that the calculation "(0x80
<< (i*8))" appears to be performing an int calculation, rather than a long
calculation. So what happens is that when you shift 0x80 by 4, the result is
0x80, because the calculation wraps around in 32 bits. This problem can be
fixed by changing 0x80 to 0x80L, but there are still problems, because the
algorithm appears to be looking for the most significant byte that has its
most significant bit set, even though the presence of any other bit set to
one in a byte would imply that the first byte of the encoding has been found
(and we may still need to append a zero byte, if the most significant bit of
the first byte is set). There are a couple of ways to solve this. One
alternative would be based on the code in the encodeUnsignedInt64 method of
the BER class:

  int len;
  for (len = 8; len > 1; len--) {
    if (((value >> (8 * (len - 1))) & 0xFF) != 0) {
      break;
    }
  }
  if ((( value >> (8 * (len -1))) & 0x080) !=0) {
    return len + 3;
  }
  return len+2;

Another alternative, which is less concise but may be quicker to execute
(not so many shift, multiply, subtract, & and operations) is based on the
logic in the getBERLength method in the UnsignedInteger32 class:

  if (value < 0x80l) {
    return 3;
  }
  else if (value < 0x8000l) {
    return 4;
  }
  else if (value < 0x800000l) {
    return 5;
  }
  else if (value < 0x80000000l) {
    return 6;
  }
  else if (value < 0x8000000000l) {
    return 7;
  }
  else if (value < 0x800000000000l) {
    return 8;
  }
  else if (value < 0x80000000000000l) {
    return 9;
  }
  else if (value < 0x8000000000000000l) {
    return 10;
  }
  return 11;

Regards,

Jim Doble
Tavve Software Company