Bug 526231 NSC_GetAttributeValue returns CKR_GENERAL_ERROR instead of CKR_ATTRIBUTE_TYPE_INVALID using the new sqlite3 backend SOFTOKEN_3_13_BRANCH
authorrrelyea%redhat.com
Wed, 19 May 2010 23:21:23 +0000
branchSOFTOKEN_3_13_BRANCH
changeset 9658 436a457f6174a851c3ce213d87cf006ed384f343
parent 9653 19500c0dbfb2f76d664dc89a7262f8017558c4f0
child 9663 caf317413eba949a60624e59558d37b78011443d
push idunknown
push userunknown
push dateunknown
bugs526231
Bug 526231 NSC_GetAttributeValue returns CKR_GENERAL_ERROR instead of CKR_ATTRIBUTE_TYPE_INVALID using the new sqlite3 backend Patch by Alex Dupre and Nelson. reviewed by rrelyea.
security/nss/lib/softoken/sdb.c
--- a/security/nss/lib/softoken/sdb.c
+++ b/security/nss/lib/softoken/sdb.c
@@ -822,82 +822,81 @@ sdb_GetAttributeValueNoLock(SDB *sdb, CK
 
 
     /* open a new db if necessary */
     error = sdb_openDBLocal(sdb_p, &sqlDB, &table);
     if (error != CKR_OK) {
 	goto loser;
     }
 
-    getStr = sqlite3_mprintf("");
-    for (i=0; getStr && i < count; i++) {
-	if (i==0) {
-	    newStr = sqlite3_mprintf("a%x", template[i].type);
-	} else {
-	    newStr = sqlite3_mprintf("%s, a%x", getStr, template[i].type);
+    for (i=0; i < count; i++) {
+	getStr = sqlite3_mprintf("a%x", template[i].type);
+
+	if (getStr == NULL) {
+	    error = CKR_HOST_MEMORY;
+	    goto loser;
 	}
+
+	newStr = sqlite3_mprintf(GET_ATTRIBUTE_CMD, getStr, table);
 	sqlite3_free(getStr);
-	getStr = newStr;
-    }
-
-    if (getStr == NULL) {
-	error = CKR_HOST_MEMORY;
-	goto loser;
-    }
+	getStr = NULL;
+	if (newStr == NULL) {
+	    error = CKR_HOST_MEMORY;
+	    goto loser;
+	}
 
-    newStr = sqlite3_mprintf(GET_ATTRIBUTE_CMD, getStr, table);
-    sqlite3_free(getStr);
-    getStr = NULL;
-    if (newStr == NULL) {
-	error = CKR_HOST_MEMORY;
-	goto loser;
-    }
+	sqlerr = sqlite3_prepare_v2(sqlDB, newStr, -1, &stmt, NULL);
+	if (sqlerr == SQLITE_ERROR) {
+	    template[i].ulValueLen = -1;
+	    error = CKR_ATTRIBUTE_TYPE_INVALID;
+	    continue;
+	} else if (sqlerr != SQLITE_OK) { goto loser; }
 
-    sqlerr = sqlite3_prepare_v2(sqlDB, newStr, -1, &stmt, NULL);
-    if (sqlerr != SQLITE_OK) { goto loser; }
-    sqlerr = sqlite3_bind_int(stmt, 1, object_id);
-    if (sqlerr != SQLITE_OK) { goto loser; }
-    do {
-	sqlerr = sqlite3_step(stmt);
-	if (sqlerr == SQLITE_BUSY) {
-	    PR_Sleep(SDB_BUSY_RETRY_TIME);
-	}
-	if (sqlerr == SQLITE_ROW) {
-	    for (i=0; i < count; i++) {
-		int column = i;
+	sqlerr = sqlite3_bind_int(stmt, 1, object_id);
+	if (sqlerr != SQLITE_OK) { goto loser; }
+
+	do {
+	    sqlerr = sqlite3_step(stmt);
+	    if (sqlerr == SQLITE_BUSY) {
+		PR_Sleep(SDB_BUSY_RETRY_TIME);
+	    }
+	    if (sqlerr == SQLITE_ROW) {
 	    	int blobSize;
 	    	const char *blobData;
 
-	    	blobSize = sqlite3_column_bytes(stmt, column);
-		blobData = sqlite3_column_blob(stmt, column);
+	    	blobSize = sqlite3_column_bytes(stmt, 0);
+		blobData = sqlite3_column_blob(stmt, 0);
 		if (blobData == NULL) {
 		    template[i].ulValueLen = -1;
 		    error = CKR_ATTRIBUTE_TYPE_INVALID; 
-		    continue;
+		    break;
 		}
 		/* If the blob equals our explicit NULL value, then the 
 		 * attribute is a NULL. */
 		if ((blobSize == SQLITE_EXPLICIT_NULL_LEN) &&
 		   	(PORT_Memcmp(blobData, SQLITE_EXPLICIT_NULL, 
 			      SQLITE_EXPLICIT_NULL_LEN) == 0)) {
 		    blobSize = 0;
 		}
 		if (template[i].pValue) {
 		    if (template[i].ulValueLen < blobSize) {
 			template[i].ulValueLen = -1;
 		    	error = CKR_BUFFER_TOO_SMALL;
-			continue;
+			break;
 		    }
 	    	    PORT_Memcpy(template[i].pValue, blobData, blobSize);
 		}
 		template[i].ulValueLen = blobSize;
+		found = 1;
 	    }
-	    found = 1;
-	}
-    } while (!sdb_done(sqlerr,&retry));
+	} while (!sdb_done(sqlerr,&retry));
+	sqlite3_reset(stmt);
+	sqlite3_finalize(stmt);
+	stmt = NULL;
+    }
 
 loser:
     /* fix up the error if necessary */
     if (error == CKR_OK) {
 	error = sdb_mapSQLError(sdb_p->type, sqlerr);
 	if (!found && error == CKR_OK) {
 	    error = CKR_OBJECT_HANDLE_INVALID;
 	}