From af3eea98557262c7ee3f432996edaed961a74b53 Mon Sep 17 00:00:00 2001 From: scottmk Date: Wed, 30 Jun 2010 13:32:28 +0000 Subject: [PATCH] 1. Degrade gracefully when the database connection dies. 2. Validate the user-specified operator in a series expression. M Open-ILS/include/openils/oils_buildq.h M Open-ILS/src/c-apps/oils_qstore.c M Open-ILS/src/c-apps/oils_buildq.c M Open-ILS/src/c-apps/oils_storedq.c M Open-ILS/src/c-apps/oils_execsql.c git-svn-id: svn://svn.open-ils.org/ILS/trunk@16834 dcc99617-32d9-48b4-a31d-7c20da2025e4 --- Open-ILS/include/openils/oils_buildq.h | 1 + Open-ILS/src/c-apps/oils_buildq.c | 1 + Open-ILS/src/c-apps/oils_execsql.c | 4 ++++ Open-ILS/src/c-apps/oils_qstore.c | 19 +++++++++++++-- Open-ILS/src/c-apps/oils_storedq.c | 32 ++++++++++++++++++++++++++ 5 files changed, 55 insertions(+), 2 deletions(-) diff --git a/Open-ILS/include/openils/oils_buildq.h b/Open-ILS/include/openils/oils_buildq.h index 8f1a3cc44d..a51c0209ad 100644 --- a/Open-ILS/include/openils/oils_buildq.h +++ b/Open-ILS/include/openils/oils_buildq.h @@ -68,6 +68,7 @@ struct BuildSQLState_ { int defaults_usable; /**< Boolean; if true, we can use unconfirmed default values for bind variables */ int values_required; /**< Boolean: if true, we need values for a bind variables */ + int panic; /**< Boolean: set to true if database connection dies */ }; typedef enum { diff --git a/Open-ILS/src/c-apps/oils_buildq.c b/Open-ILS/src/c-apps/oils_buildq.c index 2f8fd23d51..e0d5059db0 100644 --- a/Open-ILS/src/c-apps/oils_buildq.c +++ b/Open-ILS/src/c-apps/oils_buildq.c @@ -35,6 +35,7 @@ BuildSQLState* buildSQLStateNew( dbi_conn dbhandle ) { state->indent = 0; state->defaults_usable = 0; state->values_required = 0; + state->panic = 0; return state; } diff --git a/Open-ILS/src/c-apps/oils_execsql.c b/Open-ILS/src/c-apps/oils_execsql.c index 0f55209fcf..4c10ddc596 100644 --- a/Open-ILS/src/c-apps/oils_execsql.c +++ b/Open-ILS/src/c-apps/oils_execsql.c @@ -10,6 +10,8 @@ #include "opensrf/log.h" #include "opensrf/string_array.h" #include "opensrf/osrf_json.h" +#include "opensrf/osrf_application.h" +#include "openils/oils_sql.h" #include "openils/oils_buildq.h" static jsonObject* get_row( BuildSQLState* state ); @@ -49,6 +51,8 @@ jsonObject* oilsFirstRow( BuildSQLState* state ) { (void) dbi_conn_error( state->dbhandle, &msg ); osrfLogError( OSRF_LOG_MARK, sqlAddMsg( state, "Unable to execute query: %s",msg ? msg : "No description available" )); + if( ! oilsIsDBConnected( state->dbhandle )) + state->panic = 1; return NULL; } diff --git a/Open-ILS/src/c-apps/oils_qstore.c b/Open-ILS/src/c-apps/oils_qstore.c index 27d88a1ad6..36b9f0797c 100644 --- a/Open-ILS/src/c-apps/oils_qstore.c +++ b/Open-ILS/src/c-apps/oils_qstore.c @@ -204,6 +204,11 @@ int doPrepare( osrfMethodContext* ctx ) { osrfLogWarning( OSRF_LOG_MARK, "Unable to load stored query # %d", query_id ); osrfAppSessionStatus( ctx->session, OSRF_STATUS_BADREQUEST, "osrfMethodException", ctx->request, "Unable to load stored query" ); + if( state->panic ) { + osrfLogError( OSRF_LOG_MARK, sqlAddMsg( state, + "Database connection isn't working" )); + osrfAppSessionPanic( ctx->session ); + } return -1; } @@ -211,8 +216,8 @@ int doPrepare( osrfMethodContext* ctx ) { osrfLogInfo( OSRF_LOG_MARK, "Token for query id # %d is \"%s\"", query_id, token ); - // Build an object to return: a hash containing the query token - // and a list of bind variables. + // Build an object to return. It will be a hash containing the query token and a + // list of bind variables. jsonObject* returned_obj = jsonNewObjectType( JSON_HASH ); jsonObjectSetKey( returned_obj, "token", jsonNewObject( token )); jsonObjectSetKey( returned_obj, "bind_variables", @@ -261,6 +266,11 @@ int doColumns( osrfMethodContext* ctx ) { if( query->state->error ) { osrfAppSessionStatus( ctx->session, OSRF_STATUS_BADREQUEST, "osrfMethodException", ctx->request, "Unable to get column names" ); + if( query->state->panic ) { + osrfLogError( OSRF_LOG_MARK, sqlAddMsg( query->state, + "Database connection isn't working" )); + osrfAppSessionPanic( ctx->session ); + } return -1; } else { osrfAppRespondComplete( ctx, col_list ); @@ -449,6 +459,11 @@ int doExecute( osrfMethodContext* ctx ) { "Unable to execute SQL statement for query id # %d", query->query->id )); osrfAppSessionStatus( ctx->session, OSRF_STATUS_BADREQUEST, "osrfMethodException", ctx->request, "Unable to execute SQL statement" ); + if( query->state->panic ) { + osrfLogError( OSRF_LOG_MARK, sqlAddMsg( query->state, + "Database connection isn't working" )); + osrfAppSessionPanic( ctx->session ); + } return -1; } diff --git a/Open-ILS/src/c-apps/oils_storedq.c b/Open-ILS/src/c-apps/oils_storedq.c index 5ff77649dc..f83128874a 100644 --- a/Open-ILS/src/c-apps/oils_storedq.c +++ b/Open-ILS/src/c-apps/oils_storedq.c @@ -139,6 +139,8 @@ jsonObject* oilsGetColNames( BuildSQLState* state, StoredQ* query ) { "Unable to execute dummy query for column names: #%d %s", errnum, msg ? msg : "No description available" )); state->error = 1; + if( ! oilsIsDBConnected( state->dbhandle )) + state->panic = 1; return NULL; } @@ -210,6 +212,8 @@ StoredQ* getStoredQuery( BuildSQLState* state, int query_id ) { "Unable to query query.stored_query table: #%d %s", errnum, msg ? msg : "No description available" )); state->error = 1; + if( ! oilsIsDBConnected( state->dbhandle )) + state->panic = 1; } pop_id( &state->query_stack ); @@ -428,6 +432,8 @@ static QSeq* loadChildQueries( BuildSQLState* state, int parent_id, const char* osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state, "%s query # %d has no child queries within it", type_str, parent_id )); state->error = 1; + if( ! oilsIsDBConnected( state->dbhandle )) + state->panic = 1; return NULL; } } else { @@ -605,6 +611,8 @@ static FromRelation* getFromRelation( BuildSQLState* state, int id ) { "Unable to query query.from_relation table: #%d %s", errnum, msg ? msg : "No description available" )); state->error = 1; + if( ! oilsIsDBConnected( state->dbhandle )) + state->panic = 1; } if( fr ) @@ -838,6 +846,8 @@ static FromRelation* getJoinList( BuildSQLState* state, int id ) { "Unable to query query.from_relation table for join list: #%d %s", errnum, msg ? msg : "No description available" )); state->error = 1; + if( ! oilsIsDBConnected( state->dbhandle )) + state->panic = 1; } return join_list; @@ -940,6 +950,8 @@ static SelectItem* getSelectList( BuildSQLState* state, int query_id ) { "Unable to query query.select_list table: #%d %s", errnum, msg ? msg : "No description available" )); state->error = 1; + if( ! oilsIsDBConnected( state->dbhandle )) + state->panic = 1; } return select_list; @@ -1071,6 +1083,8 @@ static BindVar* getBindVar( BuildSQLState* state, const char* name ) { "Unable to query query.bind_variable table for \"%s\": #%d %s", name, errnum, msg ? msg : "No description available" )); state->error = 1; + if( ! oilsIsDBConnected( state->dbhandle )) + state->panic = 1; } if( bind ) { @@ -1251,6 +1265,8 @@ static CaseBranch* getCaseBranchList( BuildSQLState* state, int parent_id ) { "Unable to query query.case_branch table for parent expression # %d: %s", parent_id, errnum, msg ? msg : "No description available" )); state->error = 1; + if( ! oilsIsDBConnected( state->dbhandle )) + state->panic = 1; } return branch_list; @@ -1377,6 +1393,8 @@ static Datatype* getDatatype( BuildSQLState* state, int id ) { "Unable to query query.datatype table: #%d %s", errnum, msg ? msg : "No description available" )); state->error = 1; + if( ! oilsIsDBConnected( state->dbhandle )) + state->panic = 1; } return datatype; } @@ -1506,6 +1524,8 @@ static Expression* getExpression( BuildSQLState* state, int id ) { "Unable to query query.expression table: #%d %s", errnum, msg ? msg : "No description available" )); state->error = 1; + if( ! oilsIsDBConnected( state->dbhandle )) + state->panic = 1; } pop_id( &state->expr_stack ); @@ -1929,6 +1949,14 @@ static Expression* constructExpression( BuildSQLState* state, dbi_result result "Series expression is empty in expression # %d", id )); state->error = 1; return NULL; + } else if( operator && !is_good_operator( operator )) { + // The specified operator contains one or more characters that aren't allowed + // in an operator. This isn't a true validation; it's just a protective + // measure to prevent certain kinds of sql injection. + osrfLogWarning( OSRF_LOG_MARK, sqlAddMsg( state, + "Series expression # %d contains invalid operator \"%s\"", id, operator )); + state->error = 1; + return NULL; } } else if( EXP_STRING == type ) { @@ -2118,6 +2146,8 @@ static Expression* getExpressionList( BuildSQLState* state, int id ) { "Unable to query query.expression table for expression list: #%d %s", errnum, msg ? msg : "No description available" )); state->error = 1; + if( ! oilsIsDBConnected( state->dbhandle )) + state->panic = 1; } return exp_list; @@ -2166,6 +2196,8 @@ static OrderItem* getOrderByList( BuildSQLState* state, int query_id ) { "Unable to query query.order_by_list table: #%d %s", errnum, msg ? msg : "No description available" )); state->error = 1; + if( ! oilsIsDBConnected( state->dbhandle )) + state->panic = 1; } return ord_list; -- 2.43.2