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

diff --git a/Open-ILS/include/openils/oils_buildq.h b/Open-ILS/include/openils/oils_buildq.h
index 8f1a3cc..a51c020 100644 (file)
--- 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 2f8fd23..e0d5059 100644 (file)
--- 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 0f55209..4c10ddc 100644 (file)
--- 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 27d88a1..36b9f07 100644 (file)
--- 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 5ff7764..f831288 100644 (file)
--- 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;