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