Derby
  1. Derby
  2. DERBY-2935

DDMReader.readLengthAndCodePoint() decodes long integer incorrectly

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.1.3.1, 10.2.2.0, 10.3.1.4, 10.4.1.3
    • Fix Version/s: 10.3.3.0, 10.4.1.3
    • Component/s: Network Server
    • Labels:
      None

      Description

      DDMReader.readLengthAndCodePoint() contains code to decode a long integer from a byte array. This code is broken since it uses int operations and not long operations in the decoding. The long might be encoded using four, six or eight bytes, and since Derby currently always uses the four bytes encoding, the bug is not exposed in the current code.

      1. test.diff
        2 kB
        Knut Anders Hatlen
      2. decode-long.diff
        2 kB
        Knut Anders Hatlen

        Activity

        Hide
        Knut Anders Hatlen added a comment -

        The attached patch fixes the decoding by forcing promotion of the operands to long. The regression tests ran cleanly.

        Show
        Knut Anders Hatlen added a comment - The attached patch fixes the decoding by forcing promotion of the operands to long. The regression tests ran cleanly.
        Hide
        Bryan Pendleton added a comment -

        Is it possible to write a new regression test, with some Long values which
        require 6 and/or 8 bytes to encode, which demonstrates the bug and the fix?

        Show
        Bryan Pendleton added a comment - Is it possible to write a new regression test, with some Long values which require 6 and/or 8 bytes to encode, which demonstrates the bug and the fix?
        Hide
        Knut Anders Hatlen added a comment -

        I don't think it's possible to test the fix with a JDBC test, at least, since Derby doesn't support data types with length >= 2^31, but it might be possible to force 6 and 8 byte values in derbynet/testProtocol.java. I'll see if I can come up with something.

        Show
        Knut Anders Hatlen added a comment - I don't think it's possible to test the fix with a JDBC test, at least, since Derby doesn't support data types with length >= 2^31, but it might be possible to force 6 and 8 byte values in derbynet/testProtocol.java. I'll see if I can come up with something.
        Hide
        Knut Anders Hatlen added a comment -

        Adding a test which used 8 bytes for the length wasn't that difficult, but since there is no way of sending data that needs the high four bytes of the length field, the test passes regardless of the fix. So I guess the best way to demonstrate the bug and the fix is by running this program:

        public class ll {
        public static void main(String[] args)

        { long l1 = ((byte) 1 & 0xFF) << 56; long l2 = ((byte) 1 & 0xFFL) << 56; System.out.println("l1 = " + l1); System.out.println("l2 = " + l2); }

        }

        Which prints:

        l1 = 16777216
        l2 = 72057594037927936

        Show
        Knut Anders Hatlen added a comment - Adding a test which used 8 bytes for the length wasn't that difficult, but since there is no way of sending data that needs the high four bytes of the length field, the test passes regardless of the fix. So I guess the best way to demonstrate the bug and the fix is by running this program: public class ll { public static void main(String[] args) { long l1 = ((byte) 1 & 0xFF) << 56; long l2 = ((byte) 1 & 0xFFL) << 56; System.out.println("l1 = " + l1); System.out.println("l2 = " + l2); } } Which prints: l1 = 16777216 l2 = 72057594037927936
        Hide
        Knut Anders Hatlen added a comment -

        Attaching a test which uses 8 bytes for the length. The test passes both with and without the fix. I'm only attaching it in case someone wants to experiment with it to find ways to expose the bug.

        Show
        Knut Anders Hatlen added a comment - Attaching a test which uses 8 bytes for the length. The test passes both with and without the fix. I'm only attaching it in case someone wants to experiment with it to find ways to expose the bug.
        Hide
        Knut Anders Hatlen added a comment -

        Committed revision 557513.

        Show
        Knut Anders Hatlen added a comment - Committed revision 557513.

          People

          • Assignee:
            Knut Anders Hatlen
            Reporter:
            Knut Anders Hatlen
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development