LP#1847805: Protect cursor name from recursive calls

In pcrud mode we were not being careful enough about when the cursor
name was created, and permission verification could clear the cursor
created in the caller.  This had the effect of disabling app-level limit
and offset calculation.

Signed-off-by: Mike Rylander <mrylander@gmail.com>

diff --git a/Open-ILS/src/c-apps/oils_sql.c b/Open-ILS/src/c-apps/oils_sql.c
index 24072c6..ff16763 100644 (file)
--- a/Open-ILS/src/c-apps/oils_sql.c
+++ b/Open-ILS/src/c-apps/oils_sql.c
@@ -5969,9 +5969,14 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_
        int flesh_depth = 0;
     int offset = 0;
     int limit = 0;
+    int can_get_cursor = 1;
+    int do_cursor = 0;
     int cursor_page_size = 50;
 
-    if (*methodtype != 'r') cursor_name = random_cursor_name(); /* Use a cursor for all but .retrieve */
+    if (cursor_name) can_get_cursor = 0; // There is already a cursor, and it is not local
+    if (*methodtype != 'r' && can_get_cursor) cursor_name = random_cursor_name(); /* Use a cursor for all but .retrieve */
+
+    if (cursor_name && can_get_cursor) do_cursor = 1;
 
        char* sql = buildSELECT( where_hash, query_hash, class_meta, ctx );
        if( !sql ) {
@@ -6019,7 +6024,7 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_
        }
 
 
-       if (cursor_name) {
+       if (do_cursor) {
                const jsonObject* limit_o = jsonObjectGetKeyConst( query_hash, "limit" );
                if( limit_o ) {
                        limit = (int) jsonObjectGetNumber( limit_o );
@@ -6114,14 +6119,14 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_
 
                rs_size = (int *) safe_malloc( sizeof(int) );   // will be freed by sessionDataFree()
                unsigned long long result_count = cursor_page_size;
-               if (!cursor_name) {
+               if (!do_cursor) {
                        result_count = dbi_result_get_numrows( result );
                }
                *rs_size = (int) result_count * (flesh_depth + 1);      // yes, we could lose some bits, but come on
                osrfHashSet( (osrfHash *) ctx->session->userData, rs_size, "rs_size_req_%d", ctx->request );
        }
 
-    if (cursor_name) {
+    if (do_cursor) {
            dbi_result_free( result ); // toss the declare result
            result = dbi_conn_queryf( dbhandle, "FETCH %d FROM \"%s\";", cursor_page_size, cursor_name );
 
@@ -6166,7 +6171,7 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_
                        } else {
                                if( !enforce_pcrud || !need_to_verify ||
                                                verifyObjectPCRUD( ctx, class_meta, row_obj, 0 /* means check user data for rs_size */ )) {
-                                       if (cursor_name) {
+                                       if (do_cursor) {
                                                if (offset > -1 && offset) {
                                                        offset--;
                                                } else {
@@ -6180,7 +6185,7 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_
                                        }
                                }
                        }
-            if (cursor_name && !cursor_page_remaining) { // fetch next page
+            if (do_cursor && !cursor_page_remaining) { // fetch next page
                 cursor_page_remaining = cursor_page_size;
                 dbi_result_free( result );
                 result = dbi_conn_queryf( dbhandle, "FETCH %d FROM \"%s\";", cursor_page_size, cursor_name );
@@ -6207,8 +6212,8 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_
                }
             }
                } while(
-            (!cursor_name && dbi_result_next_row(result)) || (
-                cursor_name && limit && ( // limit is -1 (none) or not yet decremented to 0
+            (!do_cursor && dbi_result_next_row(result)) || (
+                do_cursor && limit && ( // limit is -1 (none) or not yet decremented to 0
                     (cursor_page_remaining == cursor_page_size && dbi_result_first_row(result)) || // on a new page, and there's a row
                     (cursor_page_remaining != cursor_page_size && dbi_result_next_row(result)) // on the same page, and there's a row
                 )
@@ -6223,7 +6228,7 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_
 
        /* clean up the query */
        dbi_result_free( result );
-       if (cursor_name) {
+       if (do_cursor) {
                result = dbi_conn_queryf( dbhandle, "CLOSE \"%s\";", cursor_name );
                dbi_result_free( result );
                free_cursor_name(); // release the cursor name now so recursive fleshing calls can make their own without leaking