Uploaded image for project: 'MariaDB Connector/J'
  1. MariaDB Connector/J
  2. CONJ-31

Data loss when using PreparedStatement.setCharacterStream and PreparedStatement.setClob

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.0, 1.1.1, 1.1.2
    • Fix Version/s: 1.1.2
    • Component/s: None
    • Labels:
      None

      Description

      setCharacterStream and setClob (when a characterStream is used) uses the wrong length of the byte buffer sent to the server. It uses the character length and not the byte length. This will lead to lost characters in the database if multibyte characters are used.

      I have attached a patch with a fix for the problem. The patch also includes some new test cases to show the problem.

        Gliffy Diagrams

          Activity

          Hide
          wlad Vladislav Vaintroub added a comment -

          Thanks. It would be great to have a little more explanation of the idea the patch. It changes many lines, and I can't tell from brief looking which ones are fixing the problem, and which ones are unrelated . Would it be possible? Thanks!

          Show
          wlad Vladislav Vaintroub added a comment - Thanks. It would be great to have a little more explanation of the idea the patch. It changes many lines, and I can't tell from brief looking which ones are fixing the problem, and which ones are unrelated . Would it be possible? Thanks!
          Hide
          runebr Rune Bremnes added a comment -

          No problem.

          In ParameterWriter.write(OutputStream out, java.io.Reader reader, long length, boolean noBackslashEscapes)
          The value returned from reader.read is the number of read characters not bytes. This value was used as input to writeBytesEscaped which expects the length in bytes.

          Changed this to use the returned length in new String instead and using the length of the returned byte array as input to the writeBytesEscaped method. This should fix the setCharacterStream issue.

          For the setClob issue, MySQLPreparedStatement.setClob used:
          setParameter(parameterIndex,
          new StreamParameter(x.getAsciiStream(), ((MySQLBlob)x).length(), connection.noBackslashEscapes));

          The length returned here is the length from MySQLClob.length() and not MySQLBlob.length (it doesn't help to cast to MySQLBlob) which again is the number of characters and not bytes. Fixed this by just removing the length parameter.
          The change to MySQLBlob.getBinaryStream is to get a stream of the correct length and not padded to 1024 bytes.

          The rest is just removing an unnecessary IOException and added test cases. And updated the ersion of surefire to allow running single tests like:
          mvn test -Dtest=DriverTest#testClob3

          I hope this explains the changes.

          Show
          runebr Rune Bremnes added a comment - No problem. In ParameterWriter.write(OutputStream out, java.io.Reader reader, long length, boolean noBackslashEscapes) The value returned from reader.read is the number of read characters not bytes. This value was used as input to writeBytesEscaped which expects the length in bytes. Changed this to use the returned length in new String instead and using the length of the returned byte array as input to the writeBytesEscaped method. This should fix the setCharacterStream issue. For the setClob issue, MySQLPreparedStatement.setClob used: setParameter(parameterIndex, new StreamParameter(x.getAsciiStream(), ((MySQLBlob)x).length(), connection.noBackslashEscapes)); The length returned here is the length from MySQLClob.length() and not MySQLBlob.length (it doesn't help to cast to MySQLBlob) which again is the number of characters and not bytes. Fixed this by just removing the length parameter. The change to MySQLBlob.getBinaryStream is to get a stream of the correct length and not padded to 1024 bytes. The rest is just removing an unnecessary IOException and added test cases. And updated the ersion of surefire to allow running single tests like: mvn test -Dtest=DriverTest#testClob3 I hope this explains the changes.
          Hide
          wlad Vladislav Vaintroub added a comment -

          Great explanations. Thanks a lot!

          Show
          wlad Vladislav Vaintroub added a comment - Great explanations. Thanks a lot!

            People

            • Assignee:
              wlad Vladislav Vaintroub
              Reporter:
              runebr Rune Bremnes
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: