From: scottmk Date: Wed, 13 Oct 2010 21:12:12 +0000 (+0000) Subject: Change the treatment of ORDER BY. Except as noted, these changes X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=562035318a27337831f765823b3d3f87b984b963;p=evergreen%2Fjoelewis.git Change the treatment of ORDER BY. Except as noted, these changes apply only to methods other than json_query. 1. Allow the use of array syntax for specifying an ORDER BY clause as an array of field specifications, such as can be used in json_query. The older syntax, using a hash based on class name, is still available. 2. For json_query, using the array syntax: relax the requirement that a class be in scope. A field from an out-of-scope class will now be silently ignored. This change avoids certain problems with fleshing queries, which use the same order_by array at multiple levels. 3. For the hash syntax: relax the requirement that the class be referenced in the SELECT clause. Now it suffices that it be in scope in the generated SQL. As a result, you can now sort by a column in a joined class without artificially including that column in the SELECT list. 4. When all or part of an ORDER BY clause is expressed as a string literal: require that the string not contain any semicolons, in order to block certain kinds of SQL injection. This measure could create problems if a semicolon appears within a quoted string -- which is possible in theory but highly improbable in practice. 5. Don't include a virtual field in an ORDER BY clause. If one is specified, silently ignore it. M Open-ILS/src/c-apps/oils_sql.c git-svn-id: svn://svn.open-ils.org/ILS/branches/rel_2_0@18325 dcc99617-32d9-48b4-a31d-7c20da2025e4 --- diff --git a/Open-ILS/src/c-apps/oils_sql.c b/Open-ILS/src/c-apps/oils_sql.c index 7a5cea87ca..168fff3c40 100644 --- a/Open-ILS/src/c-apps/oils_sql.c +++ b/Open-ILS/src/c-apps/oils_sql.c @@ -90,8 +90,11 @@ static char* searchINPredicate ( const char*, osrfHash*, jsonObject*, const char*, osrfMethodContext* ); static char* searchPredicate ( const ClassInfo*, osrfHash*, jsonObject*, osrfMethodContext* ); static char* searchJOIN ( const jsonObject*, const ClassInfo* left_info ); -static char* searchWHERE ( const jsonObject*, const ClassInfo*, int, osrfMethodContext* ); -static char* buildSELECT ( jsonObject*, jsonObject*, osrfHash*, osrfMethodContext* ); +static char* searchWHERE ( const jsonObject* search_hash, const ClassInfo*, int, osrfMethodContext* ); +static char* buildSELECT( const jsonObject*, jsonObject* rest_of_query, + osrfHash* meta, osrfMethodContext* ctx ); +static char* buildOrderByFromArray( osrfMethodContext* ctx, const jsonObject* order_array ); + char* buildQuery( osrfMethodContext* ctx, jsonObject* query, int flags ); char* SELECT ( osrfMethodContext*, jsonObject*, const jsonObject*, const jsonObject*, @@ -4277,7 +4280,6 @@ char* SELECT ( jsonIteratorFree( selclass_itr ); } - char* col_list = buffer_release( select_buf ); // Make sure the SELECT list isn't empty. This can happen, for example, @@ -4392,172 +4394,23 @@ char* SELECT ( } } - growing_buffer* order_buf = NULL; // to collect ORDER BY list - // Build an ORDER BY clause, if there is one if( NULL == order_hash ) ; // No ORDER BY? do nothing else if( JSON_ARRAY == order_hash->type ) { - // Array of field specifications, each specification being a - // hash to define the class, field, and other details - int order_idx = 0; - jsonObject* order_spec; - while( (order_spec = jsonObjectGetIndex( order_hash, order_idx++ ) ) ) { - - if( JSON_HASH != order_spec->type ) { - osrfLogError( OSRF_LOG_MARK, - "%s: Malformed field specification in ORDER BY clause; expected JSON_HASH, found %s", - modulename, json_type( order_spec->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( having_buf ); - buffer_free( group_buf ); - buffer_free( sql_buf ); - if( defaultselhash ) - jsonObjectFree( defaultselhash ); - return NULL; - } - - const char* class_alias = - jsonObjectGetString( jsonObjectGetKeyConst( order_spec, "class" ) ); - const char* field = - jsonObjectGetString( jsonObjectGetKeyConst( order_spec, "field" ) ); - - if( order_buf ) - OSRF_BUFFER_ADD( order_buf, ", " ); - else - order_buf = buffer_init( 128 ); - - if( !field || !class_alias ) { - osrfLogError( OSRF_LOG_MARK, - "%s: Missing class or field name in field specification " - "of ORDER BY clause", - modulename ); - 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( having_buf ); - buffer_free( group_buf ); - buffer_free( sql_buf ); - if( defaultselhash ) - jsonObjectFree( defaultselhash ); - return NULL; - } - - ClassInfo* order_class_info = search_alias( class_alias ); - if( ! order_class_info ) { - osrfLogError( OSRF_LOG_MARK, "%s: ORDER BY clause references class \"%s\" " - "not in FROM clause", modulename, class_alias ); - if( ctx ) - osrfAppSessionStatus( - ctx->session, - OSRF_STATUS_INTERNALSERVERERROR, - "osrfMethodException", - ctx->request, - "Invalid class referenced in ORDER BY clause -- " - "see error log for more details" - ); - free( having_buf ); - buffer_free( group_buf ); - buffer_free( sql_buf ); - if( defaultselhash ) - jsonObjectFree( defaultselhash ); - return NULL; - } - - osrfHash* field_def = osrfHashGet( order_class_info->fields, field ); - if( !field_def ) { - osrfLogError( OSRF_LOG_MARK, - "%s: Invalid field \"%s\".%s referenced in ORDER BY clause", - modulename, class_alias, field ); - if( ctx ) - osrfAppSessionStatus( - ctx->session, - OSRF_STATUS_INTERNALSERVERERROR, - "osrfMethodException", - ctx->request, - "Invalid field referenced in ORDER BY clause -- " - "see error log for more details" - ); - 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, field ); - if( ctx ) - osrfAppSessionStatus( - ctx->session, - OSRF_STATUS_INTERNALSERVERERROR, - "osrfMethodException", - ctx->request, - "Virtual field in ORDER BY clause -- see error log for more details" - ); - buffer_free( order_buf ); - free( having_buf ); - buffer_free( group_buf ); - buffer_free( sql_buf ); - if( defaultselhash ) - jsonObjectFree( defaultselhash ); - return NULL; - } - - if( jsonObjectGetKeyConst( order_spec, "transform" ) ) { - char* transform_str = searchFieldTransform( - class_alias, field_def, order_spec ); - if( ! transform_str ) { - if( ctx ) - osrfAppSessionStatus( - ctx->session, - OSRF_STATUS_INTERNALSERVERERROR, - "osrfMethodException", - ctx->request, - "Severe query error in ORDER BY clause -- " - "see error log for more details" - ); - buffer_free( order_buf ); - free( having_buf ); - buffer_free( group_buf ); - buffer_free( sql_buf ); - if( defaultselhash ) - jsonObjectFree( defaultselhash ); - return NULL; - } - - OSRF_BUFFER_ADD( order_buf, transform_str ); - free( transform_str ); - } - else - buffer_fadd( order_buf, "\"%s\".%s", class_alias, field ); - - const char* direction = - jsonObjectGetString( jsonObjectGetKeyConst( order_spec, "direction" ) ); - if( direction ) { - if( direction[ 0 ] || 'D' == direction[ 0 ] ) - OSRF_BUFFER_ADD( order_buf, " DESC" ); - else - OSRF_BUFFER_ADD( order_buf, " ASC" ); - } + order_by_list = buildOrderByFromArray( ctx, order_hash ); + if( !order_by_list ) { + free( having_buf ); + buffer_free( group_buf ); + buffer_free( sql_buf ); + if( defaultselhash ) + jsonObjectFree( defaultselhash ); + return NULL; } } else if( JSON_HASH == order_hash->type ) { // This hash is keyed on class alias. Each class has either // an array of field names or a hash keyed on field name. + growing_buffer* order_buf = NULL; // to collect ORDER BY list jsonIterator* class_itr = jsonNewIterator( order_hash ); while( (snode = jsonIteratorNext( class_itr )) ) { @@ -4573,7 +4426,7 @@ char* SELECT ( "osrfMethodException", ctx->request, "Invalid class referenced in ORDER BY clause -- " - "see error log for more details" + "see error log for more details" ); jsonIteratorFree( class_itr ); buffer_free( order_buf ); @@ -4820,6 +4673,8 @@ char* SELECT ( } } // end while jsonIteratorFree( class_itr ); + if( order_buf ) + order_by_list = buffer_release( order_buf ); } else { osrfLogError( OSRF_LOG_MARK, "%s: Malformed ORDER BY clause; expected JSON_HASH or JSON_ARRAY, found %s", @@ -4832,7 +4687,6 @@ char* SELECT ( ctx->request, "Malformed ORDER BY clause -- see error log for more details" ); - buffer_free( order_buf ); free( having_buf ); buffer_free( group_buf ); buffer_free( sql_buf ); @@ -4840,12 +4694,8 @@ char* SELECT ( jsonObjectFree( defaultselhash ); return NULL; } - - if( order_buf ) - order_by_list = buffer_release( order_buf ); } - string = buffer_release( group_buf ); if( *string && ( aggregate_found || (flags & SELECT_DISTINCT) ) ) { @@ -4891,26 +4741,199 @@ char* SELECT ( } // end of SELECT() -static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrfHash* meta, osrfMethodContext* ctx ) { +/** + @brief Build a list of ORDER BY expressions. + @param ctx Pointer to the method context. + @param order_array Pointer to a JSON_ARRAY of field specifications. + @return Pointer to a string containing a comma-separated list of ORDER BY expressions. + Each expression may be either a column reference or a function call whose first parameter + is a column reference. + + Each entry in @a order_array must be a JSON_HASH with values for "class" and "field". + It may optionally include entries for "direction" and/or "transform". + + The calling code is responsible for freeing the returned string. +*/ +static char* buildOrderByFromArray( osrfMethodContext* ctx, const jsonObject* order_array ) { + if( ! order_array ) { + osrfLogError( OSRF_LOG_MARK, "%s: Logic error: NULL pointer for ORDER BY clause", + modulename ); + if( ctx ) + osrfAppSessionStatus( + ctx->session, + OSRF_STATUS_INTERNALSERVERERROR, + "osrfMethodException", + ctx->request, + "Logic error: ORDER BY clause expected, not found; " + "see error log for more details" + ); + return NULL; + } else if( order_array->type != JSON_ARRAY ) { + osrfLogError( OSRF_LOG_MARK, + "%s: Logic error: Expected JSON_ARRAY for ORDER BY clause, not found", modulename ); + if( ctx ) + osrfAppSessionStatus( + ctx->session, + OSRF_STATUS_INTERNALSERVERERROR, + "osrfMethodException", + ctx->request, + "Logic error: Unexpected format for ORDER BY clause; see error log for more details" ); + return NULL; + } + + growing_buffer* order_buf = buffer_init( 128 ); + int first = 1; // boolean + int order_idx = 0; + jsonObject* order_spec; + while( (order_spec = jsonObjectGetIndex( order_array, order_idx++ ))) { + + if( JSON_HASH != order_spec->type ) { + osrfLogError( OSRF_LOG_MARK, + "%s: Malformed field specification in ORDER BY clause; " + "expected JSON_HASH, found %s", + modulename, json_type( order_spec->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 ); + return NULL; + } + + const char* class_alias = + jsonObjectGetString( jsonObjectGetKeyConst( order_spec, "class" )); + const char* field = + jsonObjectGetString( jsonObjectGetKeyConst( order_spec, "field" )); + + if( !field || !class_alias ) { + osrfLogError( OSRF_LOG_MARK, + "%s: Missing class or field name in field specification of ORDER BY clause", + modulename ); + 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 ); + return NULL; + } + + const ClassInfo* order_class_info = search_alias( class_alias ); + if( ! order_class_info ) { + osrfLogInternal( OSRF_LOG_MARK, "%s: ORDER BY clause references class \"%s\" " + "not in FROM clause, skipping it", modulename, class_alias ); + continue; + } + + // Add a separating comma, except at the beginning + if( first ) + first = 0; + else + OSRF_BUFFER_ADD( order_buf, ", " ); + + osrfHash* field_def = osrfHashGet( order_class_info->fields, field ); + if( !field_def ) { + osrfLogError( OSRF_LOG_MARK, + "%s: Invalid field \"%s\".%s referenced in ORDER BY clause", + modulename, class_alias, field ); + if( ctx ) + osrfAppSessionStatus( + ctx->session, + OSRF_STATUS_INTERNALSERVERERROR, + "osrfMethodException", + ctx->request, + "Invalid field referenced in ORDER BY clause -- " + "see error log for more details" + ); + free( order_buf ); + return NULL; + } else if( str_is_true( osrfHashGet( field_def, "virtual" ) ) ) { + osrfLogError( OSRF_LOG_MARK, "%s: Virtual field \"%s\" in ORDER BY clause", + modulename, field ); + if( ctx ) + osrfAppSessionStatus( + ctx->session, + OSRF_STATUS_INTERNALSERVERERROR, + "osrfMethodException", + ctx->request, + "Virtual field in ORDER BY clause -- see error log for more details" + ); + buffer_free( order_buf ); + return NULL; + } + + if( jsonObjectGetKeyConst( order_spec, "transform" )) { + char* transform_str = searchFieldTransform( class_alias, field_def, order_spec ); + if( ! transform_str ) { + if( ctx ) + osrfAppSessionStatus( + ctx->session, + OSRF_STATUS_INTERNALSERVERERROR, + "osrfMethodException", + ctx->request, + "Severe query error in ORDER BY clause -- " + "see error log for more details" + ); + buffer_free( order_buf ); + return NULL; + } + + OSRF_BUFFER_ADD( order_buf, transform_str ); + free( transform_str ); + } + else + buffer_fadd( order_buf, "\"%s\".%s", class_alias, field ); + + const char* direction = + jsonObjectGetString( jsonObjectGetKeyConst( order_spec, "direction" ) ); + if( direction ) { + if( direction[ 0 ] || 'D' == direction[ 0 ] ) + OSRF_BUFFER_ADD( order_buf, " DESC" ); + else + OSRF_BUFFER_ADD( order_buf, " ASC" ); + } + } + + return buffer_release( order_buf ); +} + +/** + @brief Build a SELECT statement. + @param search_hash Pointer to a JSON_HASH or JSON_ARRAY encoding the WHERE clause. + @param rest_of_query Pointer to a JSON_HASH containing any other SQL clauses. + @param meta Pointer to the class metadata for the core class. + @param ctx Pointer to the method context. + @return Pointer to a character string containing the WHERE clause; or NULL upon error. + + Within the rest_of_query hash, the meaningful keys are "join", "select", "no_i18n", + "order_by", "limit", and "offset". + + The SELECT statements built here are distinct from those built for the json_query method. +*/ +static char* buildSELECT ( const jsonObject* search_hash, jsonObject* rest_of_query, + osrfHash* meta, osrfMethodContext* ctx ) { const char* locale = osrf_message_get_last_locale(); osrfHash* fields = osrfHashGet( meta, "fields" ); - char* core_class = osrfHashGet( meta, "classname" ); + const char* core_class = osrfHashGet( meta, "classname" ); - const jsonObject* join_hash = jsonObjectGetKeyConst( order_hash, "join" ); + const jsonObject* join_hash = jsonObjectGetKeyConst( rest_of_query, "join" ); - jsonObject* node = NULL; - jsonObject* snode = NULL; - jsonObject* onode = NULL; - const jsonObject* _tmp = NULL; jsonObject* selhash = NULL; jsonObject* defaultselhash = NULL; growing_buffer* sql_buf = buffer_init( 128 ); growing_buffer* select_buf = buffer_init( 128 ); - if( !(selhash = jsonObjectGetKey( order_hash, "select" )) ) { + if( !(selhash = jsonObjectGetKey( rest_of_query, "select" )) ) { defaultselhash = jsonNewObjectType( JSON_HASH ); selhash = defaultselhash; } @@ -4932,20 +4955,24 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf jsonObjectSetKey( selhash, core_class, field_list ); } + // Build a list of columns for the SELECT clause int first = 1; + const jsonObject* snode = NULL; jsonIterator* class_itr = jsonNewIterator( selhash ); - while( (snode = jsonIteratorNext( class_itr )) ) { + while( (snode = jsonIteratorNext( class_itr )) ) { // For each class + // If the class isn't in the IDL, ignore it const char* cname = class_itr->key; osrfHash* idlClass = osrfHashGet( oilsIDL(), cname ); if( !idlClass ) continue; - if( strcmp(core_class,class_itr->key )) { + // If the class isn't the core class, and isn't in the JOIN clause, ignore it + if( strcmp( core_class, class_itr->key )) { if( !join_hash ) continue; - jsonObject* found = jsonObjectFindPath( join_hash, "//%s", class_itr->key ); + jsonObject* found = jsonObjectFindPath( join_hash, "//%s", class_itr->key ); if( !found->size ) { jsonObjectFree( found ); continue; @@ -4954,6 +4981,7 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf jsonObjectFree( found ); } + const jsonObject* node = NULL; jsonIterator* select_itr = jsonNewIterator( snode ); while( (node = jsonIteratorNext( select_itr )) ) { const char* item_str = jsonObjectGetString( node ); @@ -4971,7 +4999,7 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf if( locale ) { const char* i18n; - const jsonObject* no_i18n_obj = jsonObjectGetKeyConst( order_hash, "no_i18n" ); + const jsonObject* no_i18n_obj = jsonObjectGetKeyConst( rest_of_query, "no_i18n" ); if( obj_is_true( no_i18n_obj ) ) // Suppress internationalization? i18n = NULL; else @@ -5019,9 +5047,13 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf ctx->request, "Unable to build query frame for core class" ); + buffer_free( sql_buf ); + if( defaultselhash ) + jsonObjectFree( defaultselhash ); return NULL; } + // Add the JOIN clauses, if any if( join_hash ) { char* join_clause = searchJOIN( join_hash, &curr_query->core ); OSRF_BUFFER_ADD_CHAR( sql_buf, ' ' ); @@ -5030,10 +5062,11 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf } osrfLogDebug( OSRF_LOG_MARK, "%s pre-predicate SQL = %s", - modulename, OSRF_BUFFER_C_STR( sql_buf )); + modulename, OSRF_BUFFER_C_STR( sql_buf )); OSRF_BUFFER_ADD( sql_buf, " WHERE " ); + // Add the conditions in the WHERE clause char* pred = searchWHERE( search_hash, &curr_query->core, AND_OP_JOIN, ctx ); if( !pred ) { osrfAppSessionStatus( @@ -5053,112 +5086,168 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf free( pred ); } - if( order_hash ) { - char* string = NULL; - if( (_tmp = jsonObjectGetKeyConst( order_hash, "order_by" )) ){ - - growing_buffer* order_buf = buffer_init( 128 ); - - first = 1; - jsonIterator* class_itr = jsonNewIterator( _tmp ); - while( (snode = jsonIteratorNext( class_itr )) ) { - - if( !jsonObjectGetKeyConst( selhash,class_itr->key )) - continue; - - if( snode->type == JSON_HASH ) { + // Add the ORDER BY, LIMIT, and/or OFFSET clauses, if present + if( rest_of_query ) { + const jsonObject* order_by = NULL; + if( ( order_by = jsonObjectGetKeyConst( rest_of_query, "order_by" )) ){ - jsonIterator* order_itr = jsonNewIterator( snode ); - while( (onode = jsonIteratorNext( order_itr )) ) { + char* order_by_list = NULL; - osrfHash* field_def = oilsIDLFindPath( "/%s/fields/%s", - class_itr->key, order_itr->key ); - if( !field_def ) - continue; + if( JSON_ARRAY == order_by->type ) { + order_by_list = buildOrderByFromArray( ctx, order_by ); + if( !order_by_list ) { + buffer_free( sql_buf ); + if( defaultselhash ) + jsonObjectFree( defaultselhash ); + clear_query_stack(); + return NULL; + } + } else if( JSON_HASH == order_by->type ) { + // We expect order_by to be a JSON_HASH keyed on class names. Traverse it + // and build a list of ORDER BY expressions. + growing_buffer* order_buf = buffer_init( 128 ); + first = 1; + jsonIterator* class_itr = jsonNewIterator( order_by ); + while( (snode = jsonIteratorNext( class_itr )) ) { // For each class: + + ClassInfo* order_class_info = search_alias( class_itr->key ); + if( ! order_class_info ) + continue; // class not referenced by FROM clause? Ignore it. + + if( JSON_HASH == snode->type ) { + + // If the data for the current class is a JSON_HASH, then it is + // keyed on field name. + + const jsonObject* onode = NULL; + jsonIterator* order_itr = jsonNewIterator( snode ); + while( (onode = jsonIteratorNext( order_itr )) ) { // For each field + + //osrfHash* field_def = oilsIDLFindPath( "/%s/fields/%s", + // class_itr->key, order_itr->key ); + osrfHash* field_def = osrfHashGet( + order_class_info->fields, order_itr->key ); + if( !field_def ) + continue; // Field not defined in IDL? Ignore it. + if( str_is_true( osrfHashGet( field_def, "virtual"))) + continue; // Field is virtual? Ignore it. + + char* field_str = NULL; + char* direction = NULL; + if( onode->type == JSON_HASH ) { + if( jsonObjectGetKeyConst( onode, "transform" ) ) { + field_str = searchFieldTransform( + class_itr->key, field_def, onode ); + if( ! field_str ) { + 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 ); + buffer_free( order_buf ); + buffer_free( sql_buf ); + if( defaultselhash ) + jsonObjectFree( defaultselhash ); + clear_query_stack(); + return NULL; + } + } else { + growing_buffer* field_buf = buffer_init( 16 ); + buffer_fadd( field_buf, "\"%s\".%s", + class_itr->key, order_itr->key ); + field_str = buffer_release( field_buf ); + } - char* direction = NULL; - if( onode->type == JSON_HASH ) { - if( jsonObjectGetKeyConst( onode, "transform" ) ) { - string = searchFieldTransform( class_itr->key, field_def, onode ); - if( ! string ) { - 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 ); - buffer_free( order_buf ); - buffer_free( sql_buf ); - if( defaultselhash ) - jsonObjectFree( defaultselhash ); - clear_query_stack(); - return NULL; + if( ( order_by = jsonObjectGetKeyConst( onode, "direction" )) ) { + const char* dir = jsonObjectGetString( order_by ); + if(!strncasecmp( dir, "d", 1 )) { + direction = " DESC"; + } } } 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 = jsonObjectGetKeyConst( onode, "direction" )) ) { - const char* dir = jsonObjectGetString( _tmp ); - if(!strncasecmp( dir, "d", 1 )) { + field_str = strdup( order_itr->key ); + const char* dir = jsonObjectGetString( onode ); + if( !strncasecmp( dir, "d", 1 )) { direction = " DESC"; + } else { + direction = " ASC"; } } - } else { - string = strdup( order_itr->key ); - const char* dir = jsonObjectGetString( onode ); - if( !strncasecmp( dir, "d", 1 )) { - direction = " DESC"; + + if( first ) { + first = 0; } else { - direction = " ASC"; + buffer_add( order_buf, ", " ); } - } - if( first ) { - first = 0; - } else { - buffer_add( order_buf, ", " ); - } - - buffer_add( order_buf, string ); - free( string ); + buffer_add( order_buf, field_str ); + free( field_str ); - if( direction ) { - buffer_add( order_buf, direction ); + if( direction ) { + buffer_add( order_buf, direction ); + } + } // end while; looping over ORDER BY expressions + + jsonIteratorFree( order_itr ); + + } else if( JSON_STRING == snode->type ) { + // We expect a comma-separated list of sort fields. + const char* str = jsonObjectGetString( snode ); + if( strchr( str, ';' )) { + // No semicolons allowed. It is theoretically possible for a + // legitimate semicolon to occur within quotes, but it's not likely + // to occur in practice in the context of an ORDER BY list. + osrfLogError( OSRF_LOG_MARK, "%s: Possible attempt at SOL injection -- " + "semicolon found in ORDER BY list: \"%s\"", modulename, str ); + if( ctx ) { + osrfAppSessionStatus( + ctx->session, + OSRF_STATUS_INTERNALSERVERERROR, + "osrfMethodException", + ctx->request, + "Possible attempt at SOL injection -- " + "semicolon found in ORDER BY list" + ); + } + jsonIteratorFree( class_itr ); + buffer_free( order_buf ); + buffer_free( sql_buf ); + if( defaultselhash ) + jsonObjectFree( defaultselhash ); + clear_query_stack(); + return NULL; } + buffer_add( order_buf, str ); + break; } - jsonIteratorFree( order_itr ); + } // end while; looping over order_by classes - } else { - const char* str = jsonObjectGetString( snode ); - buffer_add( order_buf, str ); - break; - } + jsonIteratorFree( class_itr ); + order_by_list = buffer_release( order_buf ); + } else { + osrfLogWarning( OSRF_LOG_MARK, + "\"order_by\" object in a query is not a JSON_HASH or JSON_ARRAY;" + "no ORDER BY generated" ); } - jsonIteratorFree( class_itr ); - - string = buffer_release( order_buf ); - - if( *string ) { + if( order_by_list && *order_by_list ) { OSRF_BUFFER_ADD( sql_buf, " ORDER BY " ); - OSRF_BUFFER_ADD( sql_buf, string ); + OSRF_BUFFER_ADD( sql_buf, order_by_list ); } - free( string ); + free( order_by_list ); } - if( (_tmp = jsonObjectGetKeyConst( order_hash, "limit" )) ) { - const char* str = jsonObjectGetString( _tmp ); + const jsonObject* limit = jsonObjectGetKeyConst( rest_of_query, "limit" ); + if( limit ) { + const char* str = jsonObjectGetString( limit ); buffer_fadd( sql_buf, " LIMIT %d", @@ -5166,9 +5255,9 @@ static char* buildSELECT ( jsonObject* search_hash, jsonObject* order_hash, osrf ); } - _tmp = jsonObjectGetKeyConst( order_hash, "offset" ); - if( _tmp ) { - const char* str = jsonObjectGetString( _tmp ); + const jsonObject* offset = jsonObjectGetKeyConst( rest_of_query, "offset" ); + if( offset ) { + const char* str = jsonObjectGetString( offset ); buffer_fadd( sql_buf, " OFFSET %d",