search

wankdanker / node-odbc

ODBC bindings for node
MIT License
174 stars 79 forks source link

Improper check of return codes in ODBC::GetColumnValue leads to infinite malloc loop #2

Closed theduderog closed 11 years ago

theduderog commented 11 years ago

ODBC::GetColumnValue is not prepared for return codes SQL_ERROR and SQL_INVALID_HANDLE and goes into an infinite loop allocating memory when either of those is returned.

Here's a test script that will cause it to happen. https://gist.github.com/theduderog/6426954

I'm working on Ubuntu 12.10, node 0.10.15, unixodbc 2.2.14, and libsqlliteodbc 0.91-3.

If I compile node-odbc with DEBUG enabled and UNICODE off, things work ok until I see calls to

ODBC::~ODBC ODBC::Free ODBCConnection::~ODBCConnection ODBCConnection::Free

From then on, all calls return SQL_INVALID_HANDLE

wankdanker commented 11 years ago

Thank you for pointing this out. While handling the INVALID_HANDLE errors is definitely a good thing to do, I'm not sure if the problem ends there. I would like to investigate why and how the handle is becoming invalid and prevent it. In your gist, you indicate that you think it is due to the delayed destructor. Just to help me out, what made you come to that conclusion?

wankdanker commented 11 years ago

Do you only see this terrible behavior when using describe()? I changed your test case to query(...) with everything else the same and it does not break.

wankdanker commented 11 years ago

This issue is actually caused because the result object returned by describe was not being closed properly. This is supposed to be handled automatically in odbc.js. Will have a fix out shortly

theduderog commented 11 years ago

Dan, it looks like you may have already tracked it down. Thanks!

Just in case it helps, I'll still try to answer your questions. Yes, it only happens when using describe() and not with query(). The reason I mentioned the destructor is that I notice when I run the test code in the gist that I only see the destructor (ODBCConnection::~ODBCConnection) getting called on the 11th iteration and after that the connection handle gets corrupted.

wankdanker commented 11 years ago

This should do it: https://github.com/wankdanker/node-odbc/commits/v0.5

Thank you for your help with this. Let me know if it works for you and close this if you would.

Thanks,

Dan

theduderog commented 11 years ago

That did it! Thanks very much for such a quick response.