Derby
  1. Derby
  2. DERBY-1595

Network server fails with DRDAProtocolException if a BLOB with size 2147483647 is streamed from client

    Details

    • Urgency:
      Normal

      Description

      When executing a program which inserts a BLOB of size 2GB-1, the Network server fails with DRDAProtocolException. This happens before it starts handling the actual LOB data:

      java org.apache.derby.drda.NetworkServerControl start
      Apache Derby Network Server - 10.2.0.4 alpha started and ready to accept connections on port 1527 at 2006-07-26 14:15:21.284 GMT
      Execution failed because of a Distributed Protocol Error: DRDA_Proto_SYNTAXRM; CODPNT arg = 0; Error Code Value = c
      org.apache.derby.impl.drda.DRDAProtocolException
      at org.apache.derby.impl.drda.DRDAConnThread.throwSyntaxrm(DRDAConnThread.java:441)
      at org.apache.derby.impl.drda.DDMReader.readLengthAndCodePoint(DDMReader.java:554)
      at org.apache.derby.impl.drda.DDMReader.getCodePoint(DDMReader.java:617)
      at org.apache.derby.impl.drda.DRDAConnThread.parseSQLDTA_work(DRDAConnThread.java:4072)
      at org.apache.derby.impl.drda.DRDAConnThread.parseSQLDTA(DRDAConnThread.java:3928)
      at org.apache.derby.impl.drda.DRDAConnThread.parseEXCSQLSTTobjects(DRDAConnThread.java:3806)
      at org.apache.derby.impl.drda.DRDAConnThread.parseEXCSQLSTT(DRDAConnThread.java:3640)
      at org.apache.derby.impl.drda.DRDAConnThread.processCommands(DRDAConnThread.java:928)
      at org.apache.derby.impl.drda.DRDAConnThread.run(DRDAConnThread.java:254)
      null
      org.apache.derby.impl.drda.DRDAProtocolException
      at org.apache.derby.impl.drda.DRDAConnThread.throwSyntaxrm(DRDAConnThread.java:441)
      at org.apache.derby.impl.drda.DDMReader.readLengthAndCodePoint(DDMReader.java:554)
      at org.apache.derby.impl.drda.DDMReader.getCodePoint(DDMReader.java:617)
      at org.apache.derby.impl.drda.DRDAConnThread.parseSQLDTA_work(DRDAConnThread.java:4072)
      at org.apache.derby.impl.drda.DRDAConnThread.parseSQLDTA(DRDAConnThread.java:3928)
      at org.apache.derby.impl.drda.DRDAConnThread.parseEXCSQLSTTobjects(DRDAConnThread.java:3806)
      at org.apache.derby.impl.drda.DRDAConnThread.parseEXCSQLSTT(DRDAConnThread.java:3640)
      at org.apache.derby.impl.drda.DRDAConnThread.processCommands(DRDAConnThread.java:928)
      at org.apache.derby.impl.drda.DRDAConnThread.run(DRDAConnThread.java:254)

        Issue Links

          Activity

          Hide
          Kathey Marsden added a comment -

          reopen for backport_reject label

          Show
          Kathey Marsden added a comment - reopen for backport_reject label
          Hide
          Kristian Waagan added a comment -

          Closing issue.

          Show
          Kristian Waagan added a comment - Closing issue.
          Hide
          Kristian Waagan added a comment -

          Backported to 10.6 with revision 963673.
          Since the merge isn't clean for older versions, I don't plan more backports.

          Show
          Kristian Waagan added a comment - Backported to 10.6 with revision 963673. Since the merge isn't clean for older versions, I don't plan more backports.
          Hide
          Kristian Waagan added a comment -

          Committed patch 2a to trunk with revision 958939.

          After the fix has seen some more testing, I'll backport it. The fix is client-side only, which means that older client versions can make use of it (assuming the merge is clean).

          Show
          Kristian Waagan added a comment - Committed patch 2a to trunk with revision 958939. After the fix has seen some more testing, I'll backport it. The fix is client-side only, which means that older client versions can make use of it (assuming the merge is clean).
          Hide
          Kristian Waagan added a comment -

          Regression tests ran without failures.
          I plan to commit this patch shortly.

          Show
          Kristian Waagan added a comment - Regression tests ran without failures. I plan to commit this patch shortly.
          Hide
          Kristian Waagan added a comment -

          Attached patch 2a, which fixes the int overflow issues.
          Since CLOBs that aren't transferred using layer B streaming are represented as UTF-16, this results in a maximum length of ~4 GB.

          Running regression tests.
          Patch ready for review.

          Show
          Kristian Waagan added a comment - Attached patch 2a, which fixes the int overflow issues. Since CLOBs that aren't transferred using layer B streaming are represented as UTF-16, this results in a maximum length of ~4 GB. Running regression tests. Patch ready for review.
          Hide
          Kristian Waagan added a comment -

          Linked DERBY-4706, since that issue removes some code that would otherwise have to be changed with regards to the switch from int to long.

          Show
          Kristian Waagan added a comment - Linked DERBY-4706 , since that issue removes some code that would otherwise have to be changed with regards to the switch from int to long.
          Hide
          Kristian Waagan added a comment -

          Thanks, Knut Anders.

          I committed patch 1a to trunk with revision 955540.
          If it turns out that the length field should be unsigned, we have to change more code and that's better addressed as a separate issue.

          Show
          Kristian Waagan added a comment - Thanks, Knut Anders. I committed patch 1a to trunk with revision 955540. If it turns out that the length field should be unsigned, we have to change more code and that's better addressed as a separate issue.
          Hide
          Knut Anders Hatlen added a comment -

          1) I agree that it won't cause any problems since the server knows how to handle the values both when sent as a 6-byte value and as an 8-byte value (given that the type specifier matches the actual length). It's probably fine to leave it as it is. I see that NetStatementRequest.buildPlaceholderLength() also uses 0x7F(...) so I think it's true that a higher value won't ever be attempted written as a 6-byte integer.

          2) Sounds OK, although I think the established pattern is to exclude asserts in production jars.

          Show
          Knut Anders Hatlen added a comment - 1) I agree that it won't cause any problems since the server knows how to handle the values both when sent as a 6-byte value and as an 8-byte value (given that the type specifier matches the actual length). It's probably fine to leave it as it is. I see that NetStatementRequest.buildPlaceholderLength() also uses 0x7F(...) so I think it's true that a higher value won't ever be attempted written as a 6-byte integer. 2) Sounds OK, although I think the established pattern is to exclude asserts in production jars.
          Hide
          Kristian Waagan added a comment -

          Hi Knut,

          1) Here's what I got:

          a) DRDA vol 3, page 300:
          "The length of the extended total length field must be a multiple of two bytes, and it cannot be longer than necessary to express the length of the object's data."

          b) .../client/net/Request:
          // according to Jim and some tests perfomred on Lob data,
          // the extended length bytes are signed. Assume that
          // if this is the case for Lobs, it is the case for
          // all extended length scenarios.

          It sounds reasonable to expect that the code never sends negative lengths, but:

          • negative values may be reserved for a special purpose
          • the DRDA spec may have made a wrong turn in this area (i.e "wasting space" by using signed instead of unsigned)
          • since the client only sends signed positive values that are small enough so that the most significant bit isn't used, the difference between client and server doesn't cause problems
          • if the server is correct, our client is breaking the spec for a series of ranges of LOB lengths (point a, the client uses 0x7FFFL, 0x7FFFFFFF and 0x7FFFFFFFFFFF). This doesn't matter, as the server doesn't seem to enforce point a.
            (- btw, I haven't investigated if the extended length bytes are used when transferring data from the server to the client)

          To move on with the reported issue, I have logged DERBY-4702 to track further discussion of this topic.

          2) No, I'm not. This is more like an assert, and I'd rather have it fail than to silently ignore the most significant parts of the long.

          Show
          Kristian Waagan added a comment - Hi Knut, 1) Here's what I got: a) DRDA vol 3, page 300: "The length of the extended total length field must be a multiple of two bytes, and it cannot be longer than necessary to express the length of the object's data." b) .../client/net/Request: // according to Jim and some tests perfomred on Lob data, // the extended length bytes are signed. Assume that // if this is the case for Lobs, it is the case for // all extended length scenarios. It sounds reasonable to expect that the code never sends negative lengths, but: negative values may be reserved for a special purpose the DRDA spec may have made a wrong turn in this area (i.e "wasting space" by using signed instead of unsigned) since the client only sends signed positive values that are small enough so that the most significant bit isn't used, the difference between client and server doesn't cause problems if the server is correct, our client is breaking the spec for a series of ranges of LOB lengths (point a, the client uses 0x7FFFL, 0x7FFFFFFF and 0x7FFFFFFFFFFF). This doesn't matter, as the server doesn't seem to enforce point a. (- btw, I haven't investigated if the extended length bytes are used when transferring data from the server to the client) To move on with the reported issue, I have logged DERBY-4702 to track further discussion of this topic. 2) No, I'm not. This is more like an assert, and I'd rather have it fail than to silently ignore the most significant parts of the long.
          Hide
          Knut Anders Hatlen added a comment -

          The 1a patch looks fine to me. Two questions, though:

          1) Is the value of MAX_LONG_6_BYTES_SIGNED correct? Looking at the corresponding code at the server side, it seems like it should have been 0xFFFFFFFFFFFFL, not 0x7FFFFFFFFFFFL. See for example DDMWriter.calculateExtendedLengthByteCount():

          else if (ddmSize <= 0xffffffffffffL)
          return 6;

          And DDMReader.readNetworkSixByteLong() is also coded to handle that extra bit:

          return (
          ((buffer[pos++] & 0xffL) << 40) +
          ...

          2) Are you planning to internationalize the new error message that was added?

          Show
          Knut Anders Hatlen added a comment - The 1a patch looks fine to me. Two questions, though: 1) Is the value of MAX_LONG_6_BYTES_SIGNED correct? Looking at the corresponding code at the server side, it seems like it should have been 0xFFFFFFFFFFFFL, not 0x7FFFFFFFFFFFL. See for example DDMWriter.calculateExtendedLengthByteCount(): else if (ddmSize <= 0xffffffffffffL) return 6; And DDMReader.readNetworkSixByteLong() is also coded to handle that extra bit: return ( ((buffer [pos++] & 0xffL) << 40) + ... 2) Are you planning to internationalize the new error message that was added?
          Hide
          Kristian Waagan added a comment -

          Attaching patch 1a, which is client change only.

          The number of extended length bytes depends on the length of the value to be transferred. The code is capable of choosing between 2, 4, 6 or 8 bytes. At some point an invalid change was introduced into the client, where a long (8 bytes) was written onto the wire when only 6 bytes were required for the length. The server expected 6 bytes, and this disagreement resulted in a protocol error.

          I'm not sure using 6 instead of 8 bytes matters regarding performance, but I chose to use 6 bytes because:
          o the DRDA spec may mandate this (I haven't checked)
          o the server already expects 6 bytes, using 8 would require server side changes as well and would lead to incompatibilities when using the new client driver with old server versions

          Note that this patch itself doesn't necessarily solve the reported problem, as the client code is likely to run into an int overflow. I'll address this in a separate patch.
          suites.All ran without failures, I'm running derbyall now

          Patch ready for review. I expect to commit the patch tomorrow.

          Show
          Kristian Waagan added a comment - Attaching patch 1a, which is client change only. The number of extended length bytes depends on the length of the value to be transferred. The code is capable of choosing between 2, 4, 6 or 8 bytes. At some point an invalid change was introduced into the client, where a long (8 bytes) was written onto the wire when only 6 bytes were required for the length. The server expected 6 bytes, and this disagreement resulted in a protocol error. I'm not sure using 6 instead of 8 bytes matters regarding performance, but I chose to use 6 bytes because: o the DRDA spec may mandate this (I haven't checked) o the server already expects 6 bytes, using 8 would require server side changes as well and would lead to incompatibilities when using the new client driver with old server versions Note that this patch itself doesn't necessarily solve the reported problem, as the client code is likely to run into an int overflow. I'll address this in a separate patch. suites.All ran without failures, I'm running derbyall now Patch ready for review. I expect to commit the patch tomorrow.
          Hide
          Kristian Waagan added a comment -

          The fix for this bug consists of two parts:

          • make the client and the server agree on how the length is represented.
            Currently the client writes a long (8 bytes), whereas the server expects 6 bytes.
          • switch to using long instead of int where appropriate in the client.
            There are several places where using int may result in an overflow (i.e. user data length + overhead).

          I hope to post the first patch tomorrow.

          Show
          Kristian Waagan added a comment - The fix for this bug consists of two parts: make the client and the server agree on how the length is represented. Currently the client writes a long (8 bytes), whereas the server expects 6 bytes. switch to using long instead of int where appropriate in the client. There are several places where using int may result in an overflow (i.e. user data length + overhead). I hope to post the first patch tomorrow.
          Hide
          Knut Anders Hatlen added a comment -

          Triaged for 10.5.2.

          Show
          Knut Anders Hatlen added a comment - Triaged for 10.5.2.
          Hide
          Julius Stroffek added a comment -

          Yes, I was able to reproduce the bug, I tried also other size near 2GB-1 but just 2GB-1 exploits the bug.

          I moved my attention away and wanted to return back after some time. I am unassigning the issue from myself to leave it for others who may want to work on that.

          Show
          Julius Stroffek added a comment - Yes, I was able to reproduce the bug, I tried also other size near 2GB-1 but just 2GB-1 exploits the bug. I moved my attention away and wanted to return back after some time. I am unassigning the issue from myself to leave it for others who may want to work on that.
          Hide
          Kathey Marsden added a comment -

          Julius, were you ever able to reproduce this issue? Are you actively working on it.

          Show
          Kathey Marsden added a comment - Julius, were you ever able to reproduce this issue? Are you actively working on it.
          Hide
          Julius Stroffek added a comment -

          The exception in my comment was caused by the lack of disk space. I had started the derby with traceAll and run out of disk space very fast...

          Show
          Julius Stroffek added a comment - The exception in my comment was caused by the lack of disk space. I had started the derby with traceAll and run out of disk space very fast...
          Hide
          Julius Stroffek added a comment -

          I was able to reproduce this but when I decreased the size of LOB to 2GB-2, I got a different exception only on client:

          java.sql.SQLException: A network protocol error was encountered and the connection has been terminated: the requested command encountered an unarchitected and implementation-specific condition for which there was no architected message
          at org.apache.derby.client.am.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:46)
          at org.apache.derby.client.am.SqlException.getSQLException(SqlException.java:345)
          at org.apache.derby.client.am.PreparedStatement.execute(PreparedStatement.java:1565)
          at com.sun.stroffek.Main.main(Main.java:45)
          Caused by: org.apache.derby.client.am.DisconnectException: A network protocol error was encountered and the connection has been terminated: the requested command encountered an unarchitected and implementation-specific condition for which there was no architected message
          at org.apache.derby.client.net.NetConnectionReply.parseCMDCHKRM(NetConnectionReply.java:888)
          at org.apache.derby.client.net.NetStatementReply.parseExecuteError(NetStatementReply.java:661)
          at org.apache.derby.client.net.NetStatementReply.parseEXCSQLSTTreply(NetStatementReply.java:332)
          at org.apache.derby.client.net.NetStatementReply.readExecute(NetStatementReply.java:70)
          at org.apache.derby.client.net.StatementReply.readExecute(StatementReply.java:55)
          at org.apache.derby.client.net.NetPreparedStatement.readExecute_(NetPreparedStatement.java:183)
          at org.apache.derby.client.am.PreparedStatement.readExecute(PreparedStatement.java:1788)
          at org.apache.derby.client.am.PreparedStatement.flowExecute(PreparedStatement.java:2108)
          at org.apache.derby.client.am.PreparedStatement.executeX(PreparedStatement.java:1571)
          at org.apache.derby.client.am.PreparedStatement.execute(PreparedStatement.java:1556)
          ... 1 more

          Show
          Julius Stroffek added a comment - I was able to reproduce this but when I decreased the size of LOB to 2GB-2, I got a different exception only on client: java.sql.SQLException: A network protocol error was encountered and the connection has been terminated: the requested command encountered an unarchitected and implementation-specific condition for which there was no architected message at org.apache.derby.client.am.SQLExceptionFactory.getSQLException(SQLExceptionFactory.java:46) at org.apache.derby.client.am.SqlException.getSQLException(SqlException.java:345) at org.apache.derby.client.am.PreparedStatement.execute(PreparedStatement.java:1565) at com.sun.stroffek.Main.main(Main.java:45) Caused by: org.apache.derby.client.am.DisconnectException: A network protocol error was encountered and the connection has been terminated: the requested command encountered an unarchitected and implementation-specific condition for which there was no architected message at org.apache.derby.client.net.NetConnectionReply.parseCMDCHKRM(NetConnectionReply.java:888) at org.apache.derby.client.net.NetStatementReply.parseExecuteError(NetStatementReply.java:661) at org.apache.derby.client.net.NetStatementReply.parseEXCSQLSTTreply(NetStatementReply.java:332) at org.apache.derby.client.net.NetStatementReply.readExecute(NetStatementReply.java:70) at org.apache.derby.client.net.StatementReply.readExecute(StatementReply.java:55) at org.apache.derby.client.net.NetPreparedStatement.readExecute_(NetPreparedStatement.java:183) at org.apache.derby.client.am.PreparedStatement.readExecute(PreparedStatement.java:1788) at org.apache.derby.client.am.PreparedStatement.flowExecute(PreparedStatement.java:2108) at org.apache.derby.client.am.PreparedStatement.executeX(PreparedStatement.java:1571) at org.apache.derby.client.am.PreparedStatement.execute(PreparedStatement.java:1556) ... 1 more

            People

            • Assignee:
              Kristian Waagan
              Reporter:
              Andreas Korneliussen
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development