#673 Buffer overrun error in SQLParser.parse()

[momo]

See: https://sourceforge.net/p/jtds/discussion/104388/thread/2692515d/

After years of happy jTDS use in many deployments for different clients, we ran into our first major issue with the driver a few weeks ago. Converting an Electronic Health Records (EHR) system to run on SQL Server, we started seeing cryptic parsing errors like the one below on queries that execute perfectly when run via SQL Enterprise Management Studio.

java.sql.SQLException: Invalid SQL statement or JDBC escape, terminating '}' not found.
at net.sourceforge.jtds.jdbc.SQLParser.parse(SQLParser.java:1155)
at net.sourceforge.jtds.jdbc.SQLParser.parse(SQLParser.java:156)
at net.sourceforge.jtds.jdbc.ConnectionJDBC2.nativeSQL(ConnectionJDBC2.java:2365)
at MsSql.main(MsSql.java:38)

I tracked the problem down to the fact that the driver allocates a fixed size output buffer that allows for only 256 characters of extra room for function expansion. The fact that an EHR is VERY interested in current age, days since an event, days between events, date ranges, etc. combined with the layered system it's built on (e.g., user-supplied SQL targeting virtual tables and using additional date-based virtual functions is translated into native SQL) means that the resulting SQL can include quite a few usages of {fn curdate()}. More than seven usages of this function in a single query causes the jTDS parser to hit an array out of bounds exception which is caught and translated into a puzzling error like the above.

As a simple example, try this query:

SELECT {fn curdate()}, {fn curdate()}, {fn curdate()}, {fn curdate()}, {fn curdate()}, {fn curdate()}, {fn curdate()}

If folks are interested I can supply some more reasonable, real-world sample queries that the system produces. One particularly complex example includes 76 (!!) references to {fn curdate()}.

I have temporarily worked around this limitation by handling the expansion myself in our own SQL parser, but I would much rather pass it through and let the driver do this expansion. A quick look at the code indicates that converting "out" from a char to a StringBuilder (or StringBuffer... not sure if jTDS still needs to support 1.4) should be a simple exercise. If initialized with the same extra room, I expect a StringBuilder implementation would have identical performance in the common case with the obvious advantage of expanding as much as needed. I'd be willing to do this migration and submit a patch, if you're open to taking such a change.

Thanks,
Adam

[momo]

Hi Adam,

thank you for reporting this bug. I verified the problem, created a unit test and patched the jTDS sources to fix the issue. I'm still evaluating the performance impact of using StringBuffer and some alternative approaches, but you can expect this bug to be fixed in jTDS 1.2.7, I plan to release within the next 2 months. If you require a fixed version immediately, please drop me a line.

Cheers,
momo

[AdamR]

Sounds great, momo! We can wait for a fix but would be happy to test the changes if you need it.

Best,
Adam

[momo]

Thanks for the offering, but the error can be reproduced easily and I think I'll get this fix working and verified without any hassle. Nevertheless, please feel free to test and report any problem, you encounter.

Cheers,
momo

[momo]

I committed a fix to SVN today.

Cheers,
momo