From 900fb824fe77423f4cc4b09992e3299f58aca32b Mon Sep 17 00:00:00 2001 From: alexpeshkoff Date: Thu, 31 Jul 2014 11:43:38 +0000 Subject: [PATCH] Fixed CORE-4505: Use of named cursor fails if statement was not executed. Should also fix CORE-4489, but I cannot check. --- examples/interfaces/04.print_table.cpp | 2 +- .../manage/SrpManagement.cpp | 2 +- src/dsql/dsql.cpp | 7 +++- src/include/firebird/Provider.h | 9 ++-- src/jrd/EngineInterface.h | 4 +- src/jrd/Mapping.cpp | 4 +- src/jrd/jrd.cpp | 19 ++++++--- src/remote/client/interface.cpp | 30 +++++++------ src/remote/server/server.cpp | 37 +++++++--------- src/yvalve/YObjects.h | 5 ++- src/yvalve/why.cpp | 42 ++++++------------- 11 files changed, 79 insertions(+), 82 deletions(-) diff --git a/examples/interfaces/04.print_table.cpp b/examples/interfaces/04.print_table.cpp index 6ee7bf6c11..5050216fae 100644 --- a/examples/interfaces/04.print_table.cpp +++ b/examples/interfaces/04.print_table.cpp @@ -93,7 +93,7 @@ int main() "or RDB$VIEW_SOURCE is not null"; // Do not use IStatement - just ask attachment to open cursor - curs = att->openCursor(st, tra, 0, sql, 3, NULL, NULL, NULL); + curs = att->openCursor(st, tra, 0, sql, 3, NULL, NULL, NULL, NULL); check(st, "openCursor"); meta = curs->getMetadata(st); diff --git a/src/auth/SecureRemotePassword/manage/SrpManagement.cpp b/src/auth/SecureRemotePassword/manage/SrpManagement.cpp index 811fb07391..e97e3b1391 100644 --- a/src/auth/SecureRemotePassword/manage/SrpManagement.cpp +++ b/src/auth/SecureRemotePassword/manage/SrpManagement.cpp @@ -150,7 +150,7 @@ private: Message out; Field grantor(out, MAX_SQL_IDENTIFIER_SIZE); Firebird::IResultSet* curs = att->openCursor(&s, tra, selGrantor.length(), - selGrantor.c_str(), SQL_DIALECT_V6, NULL, NULL, out.getMetadata()); + selGrantor.c_str(), SQL_DIALECT_V6, NULL, NULL, out.getMetadata(), NULL); check(&s); bool hasGrant = curs->fetchNext(&s, out.getBuffer()); diff --git a/src/dsql/dsql.cpp b/src/dsql/dsql.cpp index 5849735b86..8d74373ec1 100644 --- a/src/dsql/dsql.cpp +++ b/src/dsql/dsql.cpp @@ -426,7 +426,7 @@ void DsqlDmlRequest::setCursor(thread_db* tdbb, const TEXT* name) const size_t MAX_CURSOR_LENGTH = 132 - 1; string cursor = name; - if (cursor.hasData() && cursor[0] == '\"') + if (cursor[0] == '\"') { // Quoted cursor names eh? Strip'em. // Note that "" will be replaced with ". @@ -477,13 +477,16 @@ void DsqlDmlRequest::setCursor(thread_db* tdbb, const TEXT* name) // If there already is a cursor and its name isn't the same, ditto. // We already know there is no cursor by this name in the hash table - if (req_cursor.isEmpty()) + if (req_cursor.isEmpty() || !(req_flags & dsql_req::FLAG_OPENED_CURSOR)) { + if (req_cursor.hasData()) + req_dbb->dbb_cursors.remove(req_cursor); req_cursor = cursor; req_dbb->dbb_cursors.put(cursor, this); } else { + fb_assert(!symbol); ERRD_post(Arg::Gds(isc_sqlerr) << Arg::Num(-502) << Arg::Gds(isc_dsql_decl_err) << Arg::Gds(isc_dsql_cursor_redefined) << req_cursor); diff --git a/src/include/firebird/Provider.h b/src/include/firebird/Provider.h index 6d0cf24369..62abdd6bc8 100644 --- a/src/include/firebird/Provider.h +++ b/src/include/firebird/Provider.h @@ -141,7 +141,6 @@ public: virtual FB_BOOLEAN FB_CARG isEof(IStatus* status) = 0; virtual FB_BOOLEAN FB_CARG isBof(IStatus* status) = 0; virtual IMessageMetadata* FB_CARG getMetadata(IStatus* status) = 0; - virtual void FB_CARG setCursorName(IStatus* status, const char* name) = 0; virtual void FB_CARG close(IStatus* status) = 0; // This item is for ISC API emulation only @@ -149,7 +148,7 @@ public: // Please do not use it! virtual void FB_CARG setDelayedOutputFormat(IStatus* status, IMessageMetadata* format) = 0; }; -#define FB_RESULTSET_VERSION (FB_REFCOUNTED_VERSION + 12) +#define FB_RESULTSET_VERSION (FB_REFCOUNTED_VERSION + 11) class IStatement : public IRefCounted { @@ -186,10 +185,11 @@ public: IMessageMetadata* inMetadata, void* inBuffer, IMessageMetadata* outMetadata, void* outBuffer) = 0; virtual IResultSet* FB_CARG openCursor(IStatus* status, ITransaction* transaction, IMessageMetadata* inMetadata, void* inBuffer, IMessageMetadata* outMetadata) = 0; + virtual void FB_CARG setCursorName(IStatus* status, const char* name) = 0; virtual void FB_CARG free(IStatus* status) = 0; virtual unsigned FB_CARG getFlags(IStatus* status) = 0; }; -#define FB_STATEMENT_VERSION (FB_REFCOUNTED_VERSION + 10) +#define FB_STATEMENT_VERSION (FB_REFCOUNTED_VERSION + 11) class IRequest : public IRefCounted { @@ -253,7 +253,8 @@ public: IMessageMetadata* inMetadata, void* inBuffer, IMessageMetadata* outMetadata, void* outBuffer) = 0; virtual IResultSet* FB_CARG openCursor(IStatus* status, ITransaction* transaction, unsigned int stmtLength, const char* sqlStmt, unsigned dialect, - IMessageMetadata* inMetadata, void* inBuffer, IMessageMetadata* outMetadata) = 0; + IMessageMetadata* inMetadata, void* inBuffer, IMessageMetadata* outMetadata, + const char* cursorName) = 0; virtual IEvents* FB_CARG queEvents(IStatus* status, IEventCallback* callback, unsigned int length, const unsigned char* events) = 0; virtual void FB_CARG cancelOperation(IStatus* status, int option) = 0; diff --git a/src/jrd/EngineInterface.h b/src/jrd/EngineInterface.h index ccd8d98fc5..44f17ada61 100644 --- a/src/jrd/EngineInterface.h +++ b/src/jrd/EngineInterface.h @@ -146,7 +146,6 @@ public: virtual FB_BOOLEAN FB_CARG isEof(Firebird::IStatus* status); virtual FB_BOOLEAN FB_CARG isBof(Firebird::IStatus* status); virtual Firebird::IMessageMetadata* FB_CARG getMetadata(Firebird::IStatus* status); - virtual void FB_CARG setCursorName(Firebird::IStatus* status, const char* name); virtual void FB_CARG close(Firebird::IStatus* status); virtual void FB_CARG setDelayedOutputFormat(Firebird::IStatus* status, Firebird::IMessageMetadata* format); @@ -190,6 +189,7 @@ public: virtual JResultSet* FB_CARG openCursor(Firebird::IStatus* status, Firebird::ITransaction* transaction, Firebird::IMessageMetadata* inMetadata, void* inBuffer, Firebird::IMessageMetadata* outMetadata); + virtual void FB_CARG setCursorName(Firebird::IStatus* status, const char* name); virtual unsigned FB_CARG getFlags(Firebird::IStatus* status); public: @@ -331,7 +331,7 @@ public: virtual Firebird::IResultSet* FB_CARG openCursor(Firebird::IStatus* status, Firebird::ITransaction* transaction, unsigned int stmtLength, const char* sqlStmt, unsigned int dialect, Firebird::IMessageMetadata* inMetadata, void* inBuffer, - Firebird::IMessageMetadata* outMetadata); + Firebird::IMessageMetadata* outMetadata, const char* cursorName); virtual JEvents* FB_CARG queEvents(Firebird::IStatus* status, Firebird::IEventCallback* callback, unsigned int length, const unsigned char* events); virtual void FB_CARG cancelOperation(Firebird::IStatus* status, int option); diff --git a/src/jrd/Mapping.cpp b/src/jrd/Mapping.cpp index 816a301957..c5191abc44 100644 --- a/src/jrd/Mapping.cpp +++ b/src/jrd/Mapping.cpp @@ -278,7 +278,7 @@ public: "SELECT RDB$MAP_USING, RDB$MAP_PLUGIN, RDB$MAP_DB, RDB$MAP_FROM_TYPE, " " RDB$MAP_FROM, RDB$MAP_TO_TYPE, RDB$MAP_TO " "FROM RDB$AUTH_MAPPING", - 3, NULL, NULL, mMap.getMetadata()); + 3, NULL, NULL, mMap.getMetadata(), NULL); if (!st.isSuccess()) { if (fb_utils::containsErrorCode(st.get(), isc_dsql_relation_err)) @@ -1190,7 +1190,7 @@ RecordBuffer* MappingList::getList(thread_db* tdbb, jrd_rel* relation) "SELECT RDB$MAP_NAME, RDB$MAP_USING, RDB$MAP_PLUGIN, RDB$MAP_DB, " " RDB$MAP_FROM_TYPE, RDB$MAP_FROM, RDB$MAP_TO_TYPE, RDB$MAP_TO " "FROM RDB$AUTH_MAPPING", - 3, NULL, NULL, mMap.getMetadata()); + 3, NULL, NULL, mMap.getMetadata(), NULL); if (!st.isSuccess()) { if (!fb_utils::containsErrorCode(st.get(), isc_dsql_relation_err)) diff --git a/src/jrd/jrd.cpp b/src/jrd/jrd.cpp index ba9779000e..2f24587c40 100644 --- a/src/jrd/jrd.cpp +++ b/src/jrd/jrd.cpp @@ -4474,7 +4474,8 @@ JResultSet* FB_CARG JStatement::openCursor(IStatus* user_status, ITransaction* t IResultSet* JAttachment::openCursor(IStatus* user_status, ITransaction* apiTra, unsigned int length, const char* string, unsigned int dialect, - IMessageMetadata* inMetadata, void* inBuffer, IMessageMetadata* outMetadata) + IMessageMetadata* inMetadata, void* inBuffer, IMessageMetadata* outMetadata, + const char* cursorName) { IStatement* tmpStatement = prepare(user_status, apiTra, length, string, dialect, (outMetadata ? 0 : IStatement::PREPARE_PREFETCH_OUTPUT_PARAMETERS)); @@ -4483,6 +4484,16 @@ IResultSet* JAttachment::openCursor(IStatus* user_status, ITransaction* apiTra, return NULL; } + if (cursorName) + { + tmpStatement->setCursorName(user_status, cursorName); + if (!user_status->isSuccess()) + { + tmpStatement->release(); + return NULL; + } + } + IResultSet* rs = tmpStatement->openCursor(user_status, apiTra, inMetadata, inBuffer, outMetadata); @@ -5035,7 +5046,7 @@ ISC_UINT64 JStatement::getAffectedRecords(IStatus* userStatus) } -void JResultSet::setCursorName(IStatus* user_status, const char* cursor) +void JStatement::setCursorName(IStatus* user_status, const char* cursor) { try { @@ -5044,9 +5055,7 @@ void JResultSet::setCursorName(IStatus* user_status, const char* cursor) try { - dsql_req* req = getStatement()->getHandle(); - fb_assert(req); - req->setCursor(tdbb, cursor); + getHandle()->setCursor(tdbb, cursor); } catch (const Exception& ex) { diff --git a/src/remote/client/interface.cpp b/src/remote/client/interface.cpp index 373e4d8004..2b3e64c980 100644 --- a/src/remote/client/interface.cpp +++ b/src/remote/client/interface.cpp @@ -253,7 +253,6 @@ public: virtual FB_BOOLEAN FB_CARG isEof(IStatus* status); virtual FB_BOOLEAN FB_CARG isBof(IStatus* status); virtual IMessageMetadata* FB_CARG getMetadata(IStatus* status); - virtual void FB_CARG setCursorName(IStatus* status, const char* name); virtual void FB_CARG close(IStatus* status); virtual void FB_CARG setDelayedOutputFormat(IStatus* status, IMessageMetadata* format); @@ -308,6 +307,7 @@ public: IMessageMetadata* outMetadata, void* outBuffer); virtual ResultSet* FB_CARG openCursor(IStatus* status, ITransaction* tra, IMessageMetadata* inMetadata, void* inBuffer, IMessageMetadata* outFormat); + virtual void FB_CARG setCursorName(IStatus* status, const char* name); virtual void FB_CARG free(IStatus* status); virtual unsigned FB_CARG getFlags(IStatus* status); @@ -475,7 +475,8 @@ public: IMessageMetadata* inMetadata, void* inBuffer, IMessageMetadata* outMetadata, void* outBuffer); virtual Firebird::IResultSet* FB_CARG openCursor(IStatus* status, ITransaction* transaction, unsigned int stmtLength, const char* sqlStmt, unsigned dialect, - IMessageMetadata* inMetadata, void* inBuffer, Firebird::IMessageMetadata* outMetadata); + IMessageMetadata* inMetadata, void* inBuffer, Firebird::IMessageMetadata* outMetadata, + const char* cursorName); virtual Firebird::IEvents* FB_CARG queEvents(IStatus* status, Firebird::IEventCallback* callback, unsigned int length, const unsigned char* events); virtual void FB_CARG cancelOperation(IStatus* status, int option); @@ -1986,7 +1987,8 @@ ResultSet* Statement::openCursor(IStatus* status, Firebird::ITransaction* apiTra IResultSet* FB_CARG Attachment::openCursor(IStatus* status, ITransaction* transaction, unsigned int stmtLength, const char* sqlStmt, unsigned dialect, - IMessageMetadata* inMetadata, void* inBuffer, IMessageMetadata* outMetadata) + IMessageMetadata* inMetadata, void* inBuffer, IMessageMetadata* outMetadata, + const char* cursorName) { Statement* stmt = prepare(status, transaction, stmtLength, sqlStmt, dialect, (outMetadata ? 0 : IStatement::PREPARE_PREFETCH_OUTPUT_PARAMETERS)); @@ -2002,6 +2004,17 @@ IResultSet* FB_CARG Attachment::openCursor(IStatus* status, ITransaction* transa return NULL; } + if (cursorName) + { + stmt->setCursorName(status, cursorName); + if (!status->isSuccess()) + { + rc->release(); + stmt->release(); + return NULL; + } + } + rc->tmpStatement = true; return rc; } @@ -3070,7 +3083,7 @@ FB_BOOLEAN ResultSet::fetchRelative(IStatus* user_status, int offset, void* buff } -void ResultSet::setCursorName(IStatus* status, const char* cursor) +void Statement::setCursorName(IStatus* status, const char* cursor) { /***************************************** * @@ -3101,16 +3114,9 @@ void ResultSet::setCursorName(IStatus* status, const char* cursor) // Check and validate handles, etc. - if (!stmt) - { - (Arg::Gds(isc_dsql_cursor_err) << Arg::Gds(isc_bad_req_handle)).raise(); - } - Rsr* statement = stmt->getStatement(); + Rsr* statement = getStatement(); CHECK_HANDLE(statement, isc_bad_req_handle); - Rdb* rdb = statement->rsr_rdb; - CHECK_HANDLE(rdb, isc_bad_db_handle); - rem_port* port = rdb->rdb_port; RefMutexGuard portGuard(*port->port_sync, FB_FUNCTION); diff --git a/src/remote/server/server.cpp b/src/remote/server/server.cpp index ee046afc94..f37564d54d 100644 --- a/src/remote/server/server.cpp +++ b/src/remote/server/server.cpp @@ -2898,7 +2898,6 @@ ISC_STATUS rem_port::end_statement(P_SQLFREE* free_stmt, PACKET* sendL) return this->send_response(sendL, 0, 0, &status_vector, true); } statement->rsr_cursor = NULL; - statement->rsr_cursor_name = ""; fb_assert(statement->rsr_rtr); FB_SIZE_T pos; if (!statement->rsr_rtr->rtr_cursors.find(statement, pos)) @@ -3230,14 +3229,6 @@ ISC_STATUS rem_port::execute_statement(P_OP op, P_SQLDATA* sqldata, PACKET* send { transaction->rtr_cursors.add(statement); statement->rsr_delayed_format = !out_blr_length; - - if (statement->rsr_cursor_name.hasData()) - { - statement->rsr_cursor->setCursorName(&status_vector, statement->rsr_cursor_name.c_str()); - - if (status_vector.isSuccess()) - statement->rsr_cursor_name = ""; - } } } else @@ -4051,6 +4042,13 @@ ISC_STATUS rem_port::prepare_statement(P_SQLST * prepareL, PACKET* sendL) if (!status_vector.isSuccess()) return this->send_response(sendL, 0, 0, &status_vector, false); + if (statement->rsr_cursor_name.hasData()) + { + statement->rsr_iface->setCursorName(&status_vector, statement->rsr_cursor_name.c_str()); + if (!status_vector.isSuccess()) + return this->send_response(sendL, 0, 0, &status_vector, false); + } + LocalStatus s2; statement->rsr_iface->getInfo(&s2, infoLength, info, prepareL->p_sqlst_buffer_length, buffer); if (!s2.isSuccess()) @@ -4999,7 +4997,6 @@ static void release_transaction( Rtr* transaction) Rsr* const statement = transaction->rtr_cursors.pop(); fb_assert(statement->rsr_cursor); statement->rsr_cursor = NULL; - statement->rsr_cursor_name = ""; } for (Rtr** p = &rdb->rdb_transactions; *p; p = &(*p)->rtr_next) @@ -5405,19 +5402,17 @@ ISC_STATUS rem_port::set_cursor(P_SQLCUR * sqlcur, PACKET* sendL) getHandle(statement, sqlcur->p_sqlcur_statement); - - if (statement->rsr_cursor) - statement->rsr_cursor->setCursorName(&status_vector, name); - else + if (port_protocol < PROTOCOL_VERSION13 && statement->rsr_cursor_name.hasData() && + statement->rsr_cursor_name != name) { - if (statement->rsr_cursor_name.hasData() && statement->rsr_cursor_name != name) - { - status_vector.set((Arg::Gds(isc_dsql_decl_err) << - Arg::Gds(isc_dsql_cursor_redefined) << statement->rsr_cursor_name).value()); - } - else - statement->rsr_cursor_name = name; + status_vector.set((Arg::Gds(isc_dsql_decl_err) << + Arg::Gds(isc_dsql_cursor_redefined) << statement->rsr_cursor_name).value()); } + else + statement->rsr_cursor_name = name; + + if (statement->rsr_iface) + statement->rsr_iface->setCursorName(&status_vector, name); return this->send_response(sendL, 0, 0, &status_vector, false); } diff --git a/src/yvalve/YObjects.h b/src/yvalve/YObjects.h index f6e48c5a11..d044fb5769 100644 --- a/src/yvalve/YObjects.h +++ b/src/yvalve/YObjects.h @@ -310,7 +310,6 @@ public: virtual FB_BOOLEAN FB_CARG isEof(Firebird::IStatus* status); virtual FB_BOOLEAN FB_CARG isBof(Firebird::IStatus* status); virtual Firebird::IMessageMetadata* FB_CARG getMetadata(Firebird::IStatus* status); - virtual void FB_CARG setCursorName(Firebird::IStatus* status, const char* name); virtual void FB_CARG close(Firebird::IStatus* status); virtual void FB_CARG setDelayedOutputFormat(Firebird::IStatus* status, Firebird::IMessageMetadata* format); @@ -358,6 +357,7 @@ public: Firebird::IMessageMetadata* outMetadata, void* outBuffer); virtual Firebird::IResultSet* FB_CARG openCursor(Firebird::IStatus* status, Firebird::ITransaction* transaction, Firebird::IMessageMetadata* inMetadata, void* inBuffer, Firebird::IMessageMetadata* outMetadata); + virtual void FB_CARG setCursorName(Firebird::IStatus* status, const char* name); virtual void FB_CARG free(Firebird::IStatus* status); virtual unsigned FB_CARG getFlags(Firebird::IStatus* status); @@ -433,7 +433,8 @@ public: Firebird::IMessageMetadata* outMetadata, void* outBuffer); virtual Firebird::IResultSet* FB_CARG openCursor(Firebird::IStatus* status, Firebird::ITransaction* transaction, unsigned int stmtLength, const char* sqlStmt, unsigned int dialect, - Firebird::IMessageMetadata* inMetadata, void* inBuffer, Firebird::IMessageMetadata* outMetadata); + Firebird::IMessageMetadata* inMetadata, void* inBuffer, Firebird::IMessageMetadata* outMetadata, + const char* cursorName); virtual YEvents* FB_CARG queEvents(Firebird::IStatus* status, Firebird::IEventCallback* callback, unsigned int length, const unsigned char* eventsData); virtual void FB_CARG cancelOperation(Firebird::IStatus* status, int option); diff --git a/src/yvalve/why.cpp b/src/yvalve/why.cpp index e606acf326..46078435d8 100644 --- a/src/yvalve/why.cpp +++ b/src/yvalve/why.cpp @@ -1370,14 +1370,6 @@ namespace { fb_assert(statement->cursor); - if (cursorName.hasData()) - { - statement->cursor->setCursorName(status, cursorName.c_str()); - - if (status->isSuccess()) - cursorName = ""; - } - delayedFormat = (outMetadata == DELAYED_OUT_FORMAT); } @@ -1408,7 +1400,6 @@ namespace { Arg::StatusVector(status->get()).raise(); statement = NULL; - cursorName = ""; } } @@ -2345,13 +2336,6 @@ ISC_STATUS API_ROUTINE isc_dsql_execute2(ISC_STATUS* userStatus, FB_API_HANDLE* } fb_assert(statement->statement->cursor); - - if (statement->cursorName.hasData()) - { - statement->statement->cursor->setCursorName(&status, statement->cursorName.c_str()); - if (status.isSuccess()) - statement->cursorName = ""; - } } else { @@ -2798,18 +2782,15 @@ ISC_STATUS API_ROUTINE isc_dsql_set_cursor_name(ISC_STATUS* userStatus, FB_API_H { RefPtr statement(translateHandle(statements, stmtHandle)); - if (statement->statement && statement->statement->cursor) - statement->statement->cursor->setCursorName(&status, cursorName); - else + if (statement->cursorName.hasData() && statement->cursorName != cursorName) { - if (statement->cursorName.hasData() && statement->cursorName != cursorName) - { - (Arg::Gds(isc_dsql_decl_err) << - Arg::Gds(isc_dsql_cursor_redefined) << statement->cursorName).raise(); - } - - statement->cursorName = cursorName; + (Arg::Gds(isc_dsql_decl_err) << + Arg::Gds(isc_dsql_cursor_redefined) << statement->cursorName).raise(); } + + statement->cursorName = cursorName; + if (statement->statement) + statement->statement->setCursorName(&status, cursorName); } catch (const Exception& e) { @@ -4444,11 +4425,11 @@ void YResultSet::destroy(unsigned dstrFlags) destroy2(dstrFlags); } -void YResultSet::setCursorName(IStatus* status, const char* name) +void YStatement::setCursorName(IStatus* status, const char* name) { try { - YEntry entry(status, this); + YEntry entry(status, this); entry.next()->setCursorName(status, name); } @@ -5173,7 +5154,8 @@ void YAttachment::executeDyn(IStatus* status, ITransaction* transaction, unsigne IResultSet* YAttachment::openCursor(IStatus* status, ITransaction* transaction, unsigned int length, const char* string, unsigned int dialect, - IMessageMetadata* inMetadata, void* inBuffer, IMessageMetadata* outMetadata) + IMessageMetadata* inMetadata, void* inBuffer, IMessageMetadata* outMetadata, + const char* cursorName) { IResultSet* rs = NULL; try @@ -5185,7 +5167,7 @@ IResultSet* YAttachment::openCursor(IStatus* status, ITransaction* transaction, getNextTransaction(status, transaction, trans); rs = entry.next()->openCursor(status, trans, length, string, dialect, - inMetadata, inBuffer, outMetadata); + inMetadata, inBuffer, outMetadata, cursorName); if (!status->isSuccess()) { return NULL;