[open-ils-commits] r13023 - trunk/Open-ILS/src/c-apps (scottmk)
svn at svn.open-ils.org
svn at svn.open-ils.org
Thu Apr 30 15:32:03 EDT 2009
Author: scottmk
Date: 2009-04-30 15:32:00 -0400 (Thu, 30 Apr 2009)
New Revision: 13023
Modified:
trunk/Open-ILS/src/c-apps/oils_cstore.c
Log:
Enforce the requirement that the ORDER BY clause be represented
by a JSON_HASH. The old code would often silently ignore a
malformed ORDER BY clause.
(Note: the output of diff makes this change look spectacularly
more complicated than it really is. Because I increased the
level of indentation of a large chunk of code, diff made a lot
of spurious matches.)
Modified: trunk/Open-ILS/src/c-apps/oils_cstore.c
===================================================================
--- trunk/Open-ILS/src/c-apps/oils_cstore.c 2009-04-30 14:42:40 UTC (rev 13022)
+++ trunk/Open-ILS/src/c-apps/oils_cstore.c 2009-04-30 19:32:00 UTC (rev 13023)
@@ -3369,36 +3369,8 @@
"osrfMethodException",
ctx->request,
"Severe query error in HAVING predicate -- see error log for more details"
- );
- }
- free(core_class);
- buffer_free(having_buf);
- buffer_free(group_buf);
- buffer_free(sql_buf);
- if (defaultselhash) jsonObjectFree(defaultselhash);
- return NULL;
- }
- }
-
- growing_buffer* order_buf = NULL; // to collect ORDER BY list
-
- // Build an ORDER BY clause, if there is one
- jsonIterator* class_itr = jsonNewIterator( order_hash );
- while ( (snode = jsonIteratorNext( class_itr )) ) {
-
- if (!jsonObjectGetKeyConst(selhash,class_itr->key)) {
- osrfLogError(OSRF_LOG_MARK, "%s: Invalid class \"%s\" referenced in ORDER BY clause",
- MODULENAME, class_itr->key );
- if( ctx )
- osrfAppSessionStatus(
- ctx->session,
- OSRF_STATUS_INTERNALSERVERERROR,
- "osrfMethodException",
- ctx->request,
- "Invalid class referenced in ORDER BY clause -- see error log for more details"
);
- jsonIteratorFree( class_itr );
- buffer_free( order_buf );
+ }
free(core_class);
buffer_free(having_buf);
buffer_free(group_buf);
@@ -3406,91 +3378,133 @@
if (defaultselhash) jsonObjectFree(defaultselhash);
return NULL;
}
+ }
- osrfHash* field_list_def = oilsIDLFindPath( "/%s/fields", class_itr->key );
+ growing_buffer* order_buf = NULL; // to collect ORDER BY list
- if ( snode->type == JSON_HASH ) {
+ // Build an ORDER BY clause, if there is one
+ if( NULL == order_hash )
+ ; // No ORDER BY? do nothing
+ else if( JSON_HASH == order_hash->type )
+ {
+ jsonIterator* class_itr = jsonNewIterator( order_hash );
+ while ( (snode = jsonIteratorNext( class_itr )) ) {
- jsonIterator* order_itr = jsonNewIterator( snode );
- while ( (onode = jsonIteratorNext( order_itr )) ) {
+ if (!jsonObjectGetKeyConst(selhash,class_itr->key)) {
+ osrfLogError(OSRF_LOG_MARK, "%s: Invalid class \"%s\" referenced in ORDER BY clause",
+ MODULENAME, class_itr->key );
+ if( ctx )
+ osrfAppSessionStatus(
+ ctx->session,
+ OSRF_STATUS_INTERNALSERVERERROR,
+ "osrfMethodException",
+ ctx->request,
+ "Invalid class referenced in ORDER BY clause -- see error log for more details"
+ );
+ jsonIteratorFree( class_itr );
+ buffer_free( order_buf );
+ free(core_class);
+ buffer_free(having_buf);
+ buffer_free(group_buf);
+ buffer_free(sql_buf);
+ if (defaultselhash) jsonObjectFree(defaultselhash);
+ return NULL;
+ }
- osrfHash* field_def = osrfHashGet( field_list_def, order_itr->key );
- if( !field_def ) {
- osrfLogError(OSRF_LOG_MARK, "%s: Invalid field \"%s\" in ORDER BY clause",
- MODULENAME, order_itr->key );
- if( ctx )
- osrfAppSessionStatus(
- ctx->session,
- OSRF_STATUS_INTERNALSERVERERROR,
- "osrfMethodException",
- ctx->request,
- "Invalid field in ORDER BY clause -- see error log for more details"
- );
- jsonIteratorFree( order_itr );
- jsonIteratorFree( class_itr );
- buffer_free( order_buf );
- free(core_class);
- buffer_free(having_buf);
- buffer_free(group_buf);
- buffer_free(sql_buf);
- if (defaultselhash) jsonObjectFree(defaultselhash);
- return NULL;
- } else if( str_is_true( osrfHashGet( field_def, "virtual" ) ) ) {
- osrfLogError(OSRF_LOG_MARK, "%s: Virtual field \"%s\" in ORDER BY clause",
- MODULENAME, order_itr->key );
- if( ctx )
- osrfAppSessionStatus(
- ctx->session,
- OSRF_STATUS_INTERNALSERVERERROR,
- "osrfMethodException",
- ctx->request,
- "Virtual field in ORDER BY clause -- see error log for more details"
- );
- jsonIteratorFree( order_itr );
- jsonIteratorFree( class_itr );
- buffer_free( order_buf );
- free(core_class);
- buffer_free(having_buf);
- buffer_free(group_buf);
- buffer_free(sql_buf);
- if (defaultselhash) jsonObjectFree(defaultselhash);
- return NULL;
- }
+ osrfHash* field_list_def = oilsIDLFindPath( "/%s/fields", class_itr->key );
- const char* direction = NULL;
- if ( onode->type == JSON_HASH ) {
- if ( jsonObjectGetKeyConst( onode, "transform" ) ) {
- string = searchFieldTransform(
- class_itr->key,
- oilsIDLFindPath( "/%s/fields/%s", class_itr->key, order_itr->key ),
- onode
- );
- if( ! string ) {
- if( ctx ) osrfAppSessionStatus(
+ if ( snode->type == JSON_HASH ) {
+
+ jsonIterator* order_itr = jsonNewIterator( snode );
+ while ( (onode = jsonIteratorNext( order_itr )) ) {
+
+ osrfHash* field_def = osrfHashGet( field_list_def, order_itr->key );
+ if( !field_def ) {
+ osrfLogError(OSRF_LOG_MARK, "%s: Invalid field \"%s\" in ORDER BY clause",
+ MODULENAME, order_itr->key );
+ if( ctx )
+ osrfAppSessionStatus(
ctx->session,
OSRF_STATUS_INTERNALSERVERERROR,
"osrfMethodException",
ctx->request,
- "Severe query error in ORDER BY clause -- see error log for more details"
+ "Invalid field in ORDER BY clause -- see error log for more details"
);
- jsonIteratorFree( order_itr );
- jsonIteratorFree( class_itr );
- free(core_class);
- buffer_free(having_buf);
- buffer_free(group_buf);
- buffer_free(order_buf);
- buffer_free(sql_buf);
- if (defaultselhash) jsonObjectFree(defaultselhash);
- return NULL;
+ jsonIteratorFree( order_itr );
+ jsonIteratorFree( class_itr );
+ buffer_free( order_buf );
+ free(core_class);
+ buffer_free(having_buf);
+ buffer_free(group_buf);
+ buffer_free(sql_buf);
+ if (defaultselhash) jsonObjectFree(defaultselhash);
+ return NULL;
+ } else if( str_is_true( osrfHashGet( field_def, "virtual" ) ) ) {
+ osrfLogError(OSRF_LOG_MARK, "%s: Virtual field \"%s\" in ORDER BY clause",
+ MODULENAME, order_itr->key );
+ if( ctx )
+ osrfAppSessionStatus(
+ ctx->session,
+ OSRF_STATUS_INTERNALSERVERERROR,
+ "osrfMethodException",
+ ctx->request,
+ "Virtual field in ORDER BY clause -- see error log for more details"
+ );
+ jsonIteratorFree( order_itr );
+ jsonIteratorFree( class_itr );
+ buffer_free( order_buf );
+ free(core_class);
+ buffer_free(having_buf);
+ buffer_free(group_buf);
+ buffer_free(sql_buf);
+ if (defaultselhash) jsonObjectFree(defaultselhash);
+ return NULL;
+ }
+
+ const char* direction = NULL;
+ if ( onode->type == JSON_HASH ) {
+ if ( jsonObjectGetKeyConst( onode, "transform" ) ) {
+ string = searchFieldTransform(
+ class_itr->key,
+ oilsIDLFindPath( "/%s/fields/%s", class_itr->key, order_itr->key ),
+ onode
+ );
+ if( ! string ) {
+ if( ctx ) osrfAppSessionStatus(
+ ctx->session,
+ OSRF_STATUS_INTERNALSERVERERROR,
+ "osrfMethodException",
+ ctx->request,
+ "Severe query error in ORDER BY clause -- see error log for more details"
+ );
+ jsonIteratorFree( order_itr );
+ jsonIteratorFree( class_itr );
+ free(core_class);
+ buffer_free(having_buf);
+ buffer_free(group_buf);
+ buffer_free(order_buf);
+ buffer_free(sql_buf);
+ if (defaultselhash) jsonObjectFree(defaultselhash);
+ return NULL;
+ }
+ } else {
+ growing_buffer* field_buf = buffer_init(16);
+ buffer_fadd(field_buf, "\"%s\".%s", class_itr->key, order_itr->key);
+ string = buffer_release(field_buf);
}
+
+ if ( (tmp_const = jsonObjectGetKeyConst( onode, "direction" )) ) {
+ const char* dir = jsonObjectGetString(tmp_const);
+ if (!strncasecmp(dir, "d", 1)) {
+ direction = " DESC";
+ } else {
+ direction = " ASC";
+ }
+ }
+
} else {
- growing_buffer* field_buf = buffer_init(16);
- buffer_fadd(field_buf, "\"%s\".%s", class_itr->key, order_itr->key);
- string = buffer_release(field_buf);
- }
-
- if ( (tmp_const = jsonObjectGetKeyConst( onode, "direction" )) ) {
- const char* dir = jsonObjectGetString(tmp_const);
+ string = strdup(order_itr->key);
+ const char* dir = jsonObjectGetString(onode);
if (!strncasecmp(dir, "d", 1)) {
direction = " DESC";
} else {
@@ -3498,115 +3512,126 @@
}
}
- } else {
- string = strdup(order_itr->key);
- const char* dir = jsonObjectGetString(onode);
- if (!strncasecmp(dir, "d", 1)) {
- direction = " DESC";
- } else {
- direction = " ASC";
- }
- }
+ if ( order_buf )
+ buffer_add(order_buf, ", ");
+ else
+ order_buf = buffer_init(128);
- if ( order_buf )
- buffer_add(order_buf, ", ");
- else
- order_buf = buffer_init(128);
+ buffer_add(order_buf, string);
+ free(string);
- buffer_add(order_buf, string);
- free(string);
+ if (direction) {
+ buffer_add(order_buf, direction);
+ }
- if (direction) {
- buffer_add(order_buf, direction);
- }
+ } // end while
+ // jsonIteratorFree(order_itr);
- } // end while
- // jsonIteratorFree(order_itr);
+ } else if ( snode->type == JSON_ARRAY ) {
- } else if ( snode->type == JSON_ARRAY ) {
+ jsonIterator* order_itr = jsonNewIterator( snode );
+ while ( (onode = jsonIteratorNext( order_itr )) ) {
- jsonIterator* order_itr = jsonNewIterator( snode );
- while ( (onode = jsonIteratorNext( order_itr )) ) {
+ const char* _f = jsonObjectGetString( onode );
- const char* _f = jsonObjectGetString( onode );
+ osrfHash* field_def = osrfHashGet( field_list_def, _f );
+ if( !field_def ) {
+ osrfLogError(OSRF_LOG_MARK, "%s: Invalid field \"%s\" in ORDER BY clause",
+ MODULENAME, _f );
+ if( ctx )
+ osrfAppSessionStatus(
+ ctx->session,
+ OSRF_STATUS_INTERNALSERVERERROR,
+ "osrfMethodException",
+ ctx->request,
+ "Invalid field in ORDER BY clause -- see error log for more details"
+ );
+ jsonIteratorFree( order_itr );
+ jsonIteratorFree( class_itr );
+ buffer_free( order_buf );
+ free(core_class);
+ buffer_free(having_buf);
+ buffer_free(group_buf);
+ buffer_free(sql_buf);
+ if (defaultselhash) jsonObjectFree(defaultselhash);
+ return NULL;
+ } else if( str_is_true( osrfHashGet( field_def, "virtual" ) ) ) {
+ osrfLogError(OSRF_LOG_MARK, "%s: Virtual field \"%s\" in ORDER BY clause",
+ MODULENAME, _f );
+ if( ctx )
+ osrfAppSessionStatus(
+ ctx->session,
+ OSRF_STATUS_INTERNALSERVERERROR,
+ "osrfMethodException",
+ ctx->request,
+ "Virtual field in ORDER BY clause -- see error log for more details"
+ );
+ jsonIteratorFree( order_itr );
+ jsonIteratorFree( class_itr );
+ buffer_free( order_buf );
+ free(core_class);
+ buffer_free(having_buf);
+ buffer_free(group_buf);
+ buffer_free(sql_buf);
+ if (defaultselhash) jsonObjectFree(defaultselhash);
+ return NULL;
+ }
- osrfHash* field_def = osrfHashGet( field_list_def, _f );
- if( !field_def ) {
- osrfLogError(OSRF_LOG_MARK, "%s: Invalid field \"%s\" in ORDER BY clause",
- MODULENAME, _f );
- if( ctx )
- osrfAppSessionStatus(
- ctx->session,
- OSRF_STATUS_INTERNALSERVERERROR,
- "osrfMethodException",
- ctx->request,
- "Invalid field in ORDER BY clause -- see error log for more details"
- );
- jsonIteratorFree( order_itr );
- jsonIteratorFree( class_itr );
- buffer_free( order_buf );
- free(core_class);
- buffer_free(having_buf);
- buffer_free(group_buf);
- buffer_free(sql_buf);
- if (defaultselhash) jsonObjectFree(defaultselhash);
- return NULL;
- } else if( str_is_true( osrfHashGet( field_def, "virtual" ) ) ) {
- osrfLogError(OSRF_LOG_MARK, "%s: Virtual field \"%s\" in ORDER BY clause",
- MODULENAME, _f );
- if( ctx )
- osrfAppSessionStatus(
- ctx->session,
- OSRF_STATUS_INTERNALSERVERERROR,
- "osrfMethodException",
- ctx->request,
- "Virtual field in ORDER BY clause -- see error log for more details"
- );
- jsonIteratorFree( order_itr );
- jsonIteratorFree( class_itr );
- buffer_free( order_buf );
- free(core_class);
- buffer_free(having_buf);
- buffer_free(group_buf);
- buffer_free(sql_buf);
- if (defaultselhash) jsonObjectFree(defaultselhash);
- return NULL;
- }
+ if ( order_buf )
+ buffer_add(order_buf, ", ");
+ else
+ order_buf = buffer_init(128);
- if ( order_buf )
- buffer_add(order_buf, ", ");
- else
- order_buf = buffer_init(128);
+ buffer_add(order_buf, _f);
- buffer_add(order_buf, _f);
-
- } // end while
+ } // end while
// jsonIteratorFree(order_itr);
+
+
+ // IT'S THE OOOOOOOOOOOLD STYLE!
+ } else {
+ osrfLogError(OSRF_LOG_MARK,
+ "%s: Possible SQL injection attempt; direct order by is not allowed", MODULENAME);
+ if (ctx) {
+ osrfAppSessionStatus(
+ ctx->session,
+ OSRF_STATUS_INTERNALSERVERERROR,
+ "osrfMethodException",
+ ctx->request,
+ "Severe query error -- see error log for more details"
+ );
+ }
-
- // IT'S THE OOOOOOOOOOOLD STYLE!
- } else {
- osrfLogError(OSRF_LOG_MARK, "%s: Possible SQL injection attempt; direct order by is not allowed", MODULENAME);
- if (ctx) {
- osrfAppSessionStatus(
- ctx->session,
- OSRF_STATUS_INTERNALSERVERERROR,
- "osrfMethodException",
- ctx->request,
- "Severe query error -- see error log for more details"
- );
- }
-
- free(core_class);
- buffer_free(having_buf);
- buffer_free(group_buf);
- buffer_free(order_buf);
- buffer_free(sql_buf);
- if (defaultselhash) jsonObjectFree(defaultselhash);
- jsonIteratorFree(class_itr);
- return NULL;
- }
- } // end while
+ free(core_class);
+ buffer_free(having_buf);
+ buffer_free(group_buf);
+ buffer_free(order_buf);
+ buffer_free(sql_buf);
+ if (defaultselhash) jsonObjectFree(defaultselhash);
+ jsonIteratorFree(class_itr);
+ return NULL;
+ }
+ } // end while
+ } else {
+ osrfLogError(OSRF_LOG_MARK,
+ "%s: Malformed ORDER BY clause; expected JSON_HASH, found %s",
+ MODULENAME, json_type( order_hash->type ) );
+ if( ctx )
+ osrfAppSessionStatus(
+ ctx->session,
+ OSRF_STATUS_INTERNALSERVERERROR,
+ "osrfMethodException",
+ ctx->request,
+ "Malformed ORDER BY clause -- see error log for more details"
+ );
+ buffer_free( order_buf );
+ free(core_class);
+ buffer_free(having_buf);
+ buffer_free(group_buf);
+ buffer_free(sql_buf);
+ if (defaultselhash) jsonObjectFree(defaultselhash);
+ return NULL;
+ }
// jsonIteratorFree(class_itr);
if( order_buf )
More information about the open-ils-commits
mailing list