|
I think there is a bug somewhere around client.net.Reply.getData()
and/or client.net.NetStatementReply.parseFDODTA(), but I don't know the code well enough to say exactly what's wrong and how to fix it. Anyway, here are my observations: There is a call to Reply.getData() when ddmScalarLen_ is -1 before the test fails (according to the code coverage report at http://people.apache.org/~fuzzylogic/codecoverage/428586/, it's always -1 when getData() is called). -1 means that the DDM length is unknown because the DDM is streamed. getData() calls adjustLengths() which has this line: ddmScalarLen_ -= length; This will give ddmScalarLen_ a negative value different from -1 (-4), which is an illegal state. At a later point in the test, NetStatementReply.parseFDODTA() is called, and ddmScalarLen_ still has the illegal value -4. parseFDODTA() passes the value of ddmScalarLen_ to ensureBLayerDataInBuffer() to ensure that there is enough data to read from the buffer. Since the argument is negative, no more data is fetched into the buffer. Since parseFDODTA() has called ensureBLayerDataInBuffer(), it assumes that it has enough data and uses "fast" methods to read the data. The fast methods read data without calling ensureALayerDataInBuffer() (presumably because the caller has already called it). Since no more data was read when ensureBLayerDataInBuffer() was called, parseFastSQLDTARDdata() and getFastData() suffer from an undetected buffer underrun. The underrun causes false data to be read from the buffer, and it makes pos_ (the position of the first available byte in the buffer) greater than count_ (the position after the last available byte in the buffer). pos_ should never be greater than count_, and when Reply.shiftBuffer() is called later, this will make the length argument to System.arraycopy() negative, which is why we get an ArrayIndexOutOfBoundsException. Thanks Knut Anders for your insights on this problem. I don't totally understand all you wrote but am suitably impressed and convinced you are on the right track. I have had a really hard time trying to narrow this issue down to a smaller test case. Do you have thoughts on how that could be done?
I think your test case is pretty close. The only difference from the JUnit test, is that this line
csp.setInt(2, (short) 32); should be replaced with csp.setLong(2, 32L); Attaching a repro which reliably reproduces the ArrayIndexOutOfBoundsException in my environment. When I run the repro, I get this exception on the server:
java.lang.ClassCastException: java.lang.Integer at org.apache.derby.impl.drda.DRDAConnThread.writeFdocaVal(DRDAConnThread.java:7255) at org.apache.derby.impl.drda.DRDAConnThread.writeFDODTA(DRDAConnThread.java:6692) at org.apache.derby.impl.drda.DRDAConnThread.parseEXCSQLSTT(DRDAConnThread.java:3847) at org.apache.derby.impl.drda.DRDAConnThread.processCommands(DRDAConnThread.java:955) at org.apache.derby.impl.drda.DRDAConnThread.run(DRDAConnThread.java:275) The ClassCastException occurs because the client sends the parameter type as DRDA_TYPE_NINTEGER8 (0x17),
because the output parameter is registered as BIGINT and a long value is set with setLong. The server then does a CallableStatement.getObject() which is an Integer because the actual procedure has SMALLINT parameters. The server then attempts to cast it to a Long in DRDAConnThread.writeFdocaVal() case DRDAConstants.DRDA_TYPE_NINTEGER8: writer.writeLong(((Long) val).longValue()); break; I am not sure if a) The client should convert the parameter to DRDA_TYPE_NINTEGER before sending it. b) The server code should handle the conversion if the parameter type sent by the client does not match the actual type. I'd appreciate any insight on which approach is best. Would it work to cast the value to Number instead of Integer/Long/Float/Double/BigInteger etc in writeFdocaVal()?
The number cast seems to work ok for everything but BigDecimal as there is no Number.bigDecimalValue(), but I keep thinking it is somehow wrong to be changing the type of the output parameter value, when returning it to the client. I think there might be cases where the output parameter value might get changed when being converted. I think we need to be able to send the parameter in as one type and out as another, so that when sending output parameters, we switch on the actual type of the parameter, not on the type sent by client.
Attaching a patch for this issue. Formerly, the server would rely on the input parameter type information received from the client to determine the output parameter type. This patch changes the server to look at the parameter metadata to determine the drda type to send.
It also enables the test ParameterMappingTest for client. stat file for diff. Also neglected to say that I ran derbyall and suites.All with the patch. I got two failures which I also get in an unchanged client and seem unrelated to my change. Likely an environment issue.
After syncing up with
The new trace is below. There is no exception on the server. Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException at java.lang.System.arraycopy(Native Method) at org.apache.derby.client.net.Reply.shiftBuffer(Reply.java:107) at org.apache.derby.client.net.Reply.ensureSpaceInBufferForFill(Reply.java:153) at org.apache.derby.client.net.Reply.fill(Reply.java:165) at org.apache.derby.client.net.Reply.ensureALayerDataInBuffer(Reply.java:215) at org.apache.derby.client.net.Reply.readDssHeader(Reply.java:317) at org.apache.derby.client.net.Reply.peekCodePoint(Reply.java:1008) at org.apache.derby.client.net.NetStatementReply.parseEXCSQLSTTreply(NetStatementReply.java:324) at org.apache.derby.client.net.NetStatementReply.readExecuteCall(NetStatementReply.java:105) at org.apache.derby.client.net.StatementReply.readExecuteCall(StatementReply.java:75) at org.apache.derby.client.net.NetStatement.readExecuteCall_(NetStatement.java:176) at org.apache.derby.client.am.Statement.readExecuteCall(Statement.java:1464) at org.apache.derby.client.am.PreparedStatement.flowExecute(PreparedStatement.java:2157) at org.apache.derby.client.am.PreparedStatement.executeX(PreparedStatement.java:1577) at org.apache.derby.client.am.PreparedStatement.execute(PreparedStatement.java:1562) at d2381.main(D2381.java:39) The problem running ParameterMappingTest was an issue with my environment. It passes now. I will rerun tests and check in.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public static void main(String[] args) throws Exception {
Class.forName("org.apache.derby.jdbc.ClientDriver");
Connection conn =DriverManager.getConnection("jdbc:derby://localhost:1527/wombat;create=true;traceFile=trace.out");
Statement s = conn.createStatement();
try {
s.executeUpdate("DROP PROCEDURE PMP.TYPE_AS");
} catch (SQLException se) {}
String procSQL = "CREATE PROCEDURE PMP.TYPE_AS(" +
"IN P1 " + "SMALLINT" +
", INOUT P2 " + "SMALLINT" +
", OUT P3 " + "SMALLINT" +
") LANGUAGE JAVA PARAMETER STYLE JAVA NO SQL " +
" EXTERNAL NAME 'org.apache.derbyTesting.functionTests.util.ProcedureTest.pmap'";
s.executeUpdate(procSQL);
CallableStatement csp = conn.prepareCall("CALL PMP.TYPE_AS(?, ?, ?)");
csp.registerOutParameter(2, java.sql.Types.INTEGER);
csp.registerOutParameter(3, java.sql.Types.INTEGER);
csp.setShort(1, (short) 32);
csp.setInt(2, (short) 32);
csp.execute();
csp.getInt(2);
csp.getInt(3);
csp.registerOutParameter(2, java.sql.Types.BIGINT);
csp.registerOutParameter(3, java.sql.Types.BIGINT);
csp.setShort(1, (short) 32);
csp.setInt(2, (short) 32);
// Error should occur on execution
csp.execute();
}