From af3eea98557262c7ee3f432996edaed961a74b53 Mon Sep 17 00:00:00 2001
From: scottmk <scottmk@dcc99617-32d9-48b4-a31d-7c20da2025e4>
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.11.0