Derby
  1. Derby
  2. DERBY-4799

IllegalArgumentException when generating error message on server

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.6.1.0
    • Fix Version/s: 10.7.1.1
    • Component/s: Network Server
    • Labels:
      None
    • Issue & fix info:
      Repro attached

      Description

      If you for example try to connect to a non-existing database using the client driver, and the name of the database has 18 characters or more, and at least one of the characters in the database name is a non-ascii character, the server will throw an IllegalArgumentException when trying to send the "database not found" message back to the client.

      Example:

      ij> connect 'jdbc:derby://localhost/abcdefghijklmnopqå';
      ERROR 08006: A network protocol error was encountered and the connection has been terminated: A PROTOCOL Data Stream Syntax Error was detected. Reason: 0x12. Plaintext connection attempt to an SSL enabled server?

      Printed to the console by the server:

      Tue Sep 14 09:12:05 CEST 2010 : fromIndex(60) > toIndex(59)
      java.lang.IllegalArgumentException: fromIndex(60) > toIndex(59)
      at java.util.Arrays.rangeCheck(Arrays.java:1306)
      at java.util.Arrays.fill(Arrays.java:2567)
      at org.apache.derby.impl.drda.DDMWriter.padBytes(DDMWriter.java:1254)
      at org.apache.derby.impl.drda.DDMWriter.writeScalarPaddedBytes(DDMWriter.java:992)
      at org.apache.derby.impl.drda.DRDAConnThread.writeRDBNAM(DRDAConnThread.java:583)
      at org.apache.derby.impl.drda.DRDAConnThread.writeRDBfailure(DRDAConnThread.java:1248)
      at org.apache.derby.impl.drda.DRDAConnThread.parseDRDAConnection(DRDAConnThread.java:1194)
      at org.apache.derby.impl.drda.DRDAConnThread.processCommands(DRDAConnThread.java:968)
      at org.apache.derby.impl.drda.DRDAConnThread.run(DRDAConnThread.java:294)

      1. test.diff
        2 kB
        Knut Anders Hatlen
      2. fix.diff
        1 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Hide
          Knut Anders Hatlen added a comment -

          The test that verifies the fix went in along with the other changes for DERBY-728, so I'm closing this issue now.

          Show
          Knut Anders Hatlen added a comment - The test that verifies the fix went in along with the other changes for DERBY-728 , so I'm closing this issue now.
          Hide
          Knut Anders Hatlen added a comment -

          OK, I'll wait for those changes. Thanks.

          Show
          Knut Anders Hatlen added a comment - OK, I'll wait for those changes. Thanks.
          Hide
          Tiago R. Espinha added a comment -

          > Should I hold off committing this fix and just check in the test to verify that the problem is solved once you've finished up DERBY-4757?

          I think that's the better option. I have a few changes that aren't on the submitted patches and I've just been waiting on that discussion on the list to see if I should go forward or hold out on it.

          Show
          Tiago R. Espinha added a comment - > Should I hold off committing this fix and just check in the test to verify that the problem is solved once you've finished up DERBY-4757 ? I think that's the better option. I have a few changes that aren't on the submitted patches and I've just been waiting on that discussion on the list to see if I should go forward or hold out on it.
          Hide
          Knut Anders Hatlen added a comment -

          NetConnectionReply.parseRDBNAM() is the method that parses the RDBNAM value sent from the server, and it indeed uses the CCSID manager. The reason why it has worked is probably that all the callers of that method have called it with skip==true, so that none of them actually do any decoding of the string. They just skip the specified number of bytes.

          So I agree, changing DRDAConnThread.writeRDBNAM() so that it uses the CCSID manager would solve this bug. And that it switches to EBCDIC when talking to older clients is not a problem since they don't look at the value anyways.

          Should I hold off committing this fix and just check in the test to verify that the problem is solved once you've finished up DERBY-4757?

          Show
          Knut Anders Hatlen added a comment - NetConnectionReply.parseRDBNAM() is the method that parses the RDBNAM value sent from the server, and it indeed uses the CCSID manager. The reason why it has worked is probably that all the callers of that method have called it with skip==true, so that none of them actually do any decoding of the string. They just skip the specified number of bytes. So I agree, changing DRDAConnThread.writeRDBNAM() so that it uses the CCSID manager would solve this bug. And that it switches to EBCDIC when talking to older clients is not a problem since they don't look at the value anyways. Should I hold off committing this fix and just check in the test to verify that the problem is solved once you've finished up DERBY-4757 ?
          Hide
          Tiago R. Espinha added a comment -

          I think this is just the behavior on the server. If you look at NetConnectionRequest.buildRDBNAM(String, boolean) you'll find this call:

          (line 488)
          netAgent_.getCurrentCcsidManager().convertFromJavaString(rdbnam, netAgent_);

          Now, if we consider that up until now the only CCSID manager was of the type EbcdicCcsidManager, then the client has always been encoding the RDBNAM in EBCDIC. It is only the server, when it has to send the RDBNAM back to the client (it happens in certain occasions), that will send it in UTF-8 as seen on the method you've changed.

          I'm not sure this follows the specification and with my changes, rather than getting the length like this, I was pulling it from the CCSID manager's getByteLength() method. I reckon the override for this method requires some adjusting in the EBCDIC case, but I think this would be more correct from a DRDA perspective.

          Show
          Tiago R. Espinha added a comment - I think this is just the behavior on the server. If you look at NetConnectionRequest.buildRDBNAM(String, boolean) you'll find this call: (line 488) netAgent_.getCurrentCcsidManager().convertFromJavaString(rdbnam, netAgent_); Now, if we consider that up until now the only CCSID manager was of the type EbcdicCcsidManager, then the client has always been encoding the RDBNAM in EBCDIC. It is only the server, when it has to send the RDBNAM back to the client (it happens in certain occasions), that will send it in UTF-8 as seen on the method you've changed. I'm not sure this follows the specification and with my changes, rather than getting the length like this, I was pulling it from the CCSID manager's getByteLength() method. I reckon the override for this method requires some adjusting in the EBCDIC case, but I think this would be more correct from a DRDA perspective.
          Hide
          Knut Anders Hatlen added a comment -

          Attaching a regression test case for the bug.

          Show
          Knut Anders Hatlen added a comment - Attaching a regression test case for the bug.
          Hide
          Knut Anders Hatlen added a comment -

          I think we've been sending the contents of the DRDA commands as UTF-8 all the time, and only the command headers as EBCDIC. Not sure exactly where the line was drawn between EBCDIC and UTF-8. I was also a bit surprised that we didn't use EBCDIC here.

          I checked the latest patch on DERBY-4757 and didn't see that it touched this method. Is this something you've planned to add in the next revision of the patch? If it already uses UTF-8, I suppose it shouldn't be changed by DERBY-4757 to use EBCDIC against older clients.

          Show
          Knut Anders Hatlen added a comment - I think we've been sending the contents of the DRDA commands as UTF-8 all the time, and only the command headers as EBCDIC. Not sure exactly where the line was drawn between EBCDIC and UTF-8. I was also a bit surprised that we didn't use EBCDIC here. I checked the latest patch on DERBY-4757 and didn't see that it touched this method. Is this something you've planned to add in the next revision of the patch? If it already uses UTF-8, I suppose it shouldn't be changed by DERBY-4757 to use EBCDIC against older clients.
          Hide
          Tiago R. Espinha added a comment -

          I'm confused now. By looking at your patch, I see that you use "server.DEFAULT_ENCODING" to obtain the bytes, which is in fact UTF8.

          Shouldn't all the DRDA commands and arguments until now be using EBCDIC?

          My changes do go into this method and rather than using a .length() call, I use the CCSID manager (whichever it is currently) to obtain the length in bytes.

          Show
          Tiago R. Espinha added a comment - I'm confused now. By looking at your patch, I see that you use "server.DEFAULT_ENCODING" to obtain the bytes, which is in fact UTF8. Shouldn't all the DRDA commands and arguments until now be using EBCDIC? My changes do go into this method and rather than using a .length() call, I use the CCSID manager (whichever it is currently) to obtain the length in bytes.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks, I've linked it to DERBY-728.

          Will your changes in 10.7 make the error handling stop using writeRDBNAM()? This is not code that uses EBCDIC in the first place, so I was under the impression that it wouldn't be touched by the DERBY-4757 changes.

          For the record, the connect command in the bug description is supposed to fail, because the database isn't found. Here's what happens with the fix applied:

          ij> connect 'jdbc:derby://localhost/abcdefghijklmnopqå';
          ERROR 08004: The connection was refused because the database abcdefghijklmnopqå was not found.

          This appears to only be a problem in the case where we expect a failure. If I add create=true to the URL so that the database is created, it works fine also without the fix.

          Show
          Knut Anders Hatlen added a comment - Thanks, I've linked it to DERBY-728 . Will your changes in 10.7 make the error handling stop using writeRDBNAM()? This is not code that uses EBCDIC in the first place, so I was under the impression that it wouldn't be touched by the DERBY-4757 changes. For the record, the connect command in the bug description is supposed to fail, because the database isn't found. Here's what happens with the fix applied: ij> connect 'jdbc:derby://localhost/abcdefghijklmnopqå'; ERROR 08004: The connection was refused because the database abcdefghijklmnopqå was not found. This appears to only be a problem in the case where we expect a failure. If I add create=true to the URL so that the database is created, it works fine also without the fix.
          Hide
          Tiago R. Espinha added a comment -

          I think perhaps we should link this with DERBY-728?

          Just some thoughts on this. My changes will make it so that a 10.7 client on a 10.7 server never sees this issue. However, a 10.7 client working with an old server will still see it as it will revert to EBCDIC.

          Show
          Tiago R. Espinha added a comment - I think perhaps we should link this with DERBY-728 ? Just some thoughts on this. My changes will make it so that a 10.7 client on a 10.7 server never sees this issue. However, a 10.7 client working with an old server will still see it as it will revert to EBCDIC.
          Hide
          Knut Anders Hatlen added a comment -

          The problem appears to be that DRDAConnThread.writeRDBNAM() uses the length of the string in characters when it calls writeScalarPaddedBytes(). writeScalarPaddedBytes() expects the length in bytes, and gets confused when paddedLength is less than buf.length.

          The reason why we need at least 18 characters in the name in order to see the bug, is that the database name is always padded to at least 18 bytes (DRDA requirement).

          The attached patch (fix.diff) makes writeRDBNAM() use number of bytes instead of number of characters. I'll see if I can come up with a regression test for the bug.

          Show
          Knut Anders Hatlen added a comment - The problem appears to be that DRDAConnThread.writeRDBNAM() uses the length of the string in characters when it calls writeScalarPaddedBytes(). writeScalarPaddedBytes() expects the length in bytes, and gets confused when paddedLength is less than buf.length. The reason why we need at least 18 characters in the name in order to see the bug, is that the database name is always padded to at least 18 bytes (DRDA requirement). The attached patch (fix.diff) makes writeRDBNAM() use number of bytes instead of number of characters. I'll see if I can come up with a regression test for the bug.
          Hide
          Knut Anders Hatlen added a comment -

          JIRA garbled the database URL in the description. The database name should be "abcdefghijklmnopqå".

          Show
          Knut Anders Hatlen added a comment - JIRA garbled the database URL in the description. The database name should be "abcdefghijklmnopqå".

            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