Bug 562866 - StatementParams::NewResolve mishandles JSVAL_IS_STRING(aId). r=sdwilsh.

--HG--
extra : rebase_source : f9e1ea925fa38a26c806ba6dbddc52ef83646d55

storage/src/mozStorageAsyncStatementParams.cpp

@@ -126,16 +126,18 @@ AsyncStatementParams::NewResolve(
 )
 {
   NS_ENSURE_TRUE(mStatement, NS_ERROR_NOT_INITIALIZED);
-  // We do not throw at any point after this unless our index is out of range
-  // because we want to allow the prototype chain to be checked for the
-  // property.
-
-  PRUint32 idx;
+  // We do not throw at any point after this because we want to allow the
+  // prototype chain to be checked for the property.
 
+  bool resolved = false;
+  PRBool ok = PR_TRUE;
   if (JSVAL_IS_INT(aId)) {
-    idx = JSVAL_TO_INT(aId);
+    PRUint32 idx = JSVAL_TO_INT(aId);
     // All indexes are good because we don't know how many parameters there
     // really are.
+    ok = ::JS_DefineElement(aCtx, aScopeObj, idx, JSVAL_VOID, nsnull,
+                            nsnull, 0);
+    resolved = true;
   }
   else if (JSVAL_IS_STRING(aId)) {
     JSString *str = JSVAL_TO_STRING(aId);
@@ -145,19 +147,13 @@ AsyncStatementParams::NewResolve(
     // We are unable to tell if there's a parameter with this name and so
     // we must assume that there is.  This screws the rest of the prototype
     // chain, but people really shouldn't be depending on this anyways.
-    *_retval = ::JS_DefineUCProperty(aCtx, aScopeObj, nameChars, nameLength,
-                                     JSVAL_VOID, nsnull, nsnull, 0);
-    NS_ENSURE_TRUE(*_retval, NS_OK);
-  }
-  else {
-    // We do not handle other types.
-    return NS_OK;
+    ok = ::JS_DefineUCProperty(aCtx, aScopeObj, nameChars, nameLength,
+                               JSVAL_VOID, nsnull, nsnull, 0);
+    resolved = true;
   }
 
-  *_retval = ::JS_DefineElement(aCtx, aScopeObj, idx, JSVAL_VOID, nsnull,
-                                nsnull, 0);
-  if (*_retval)
-    *_objp = aScopeObj;
+  *_retval = ok;
+  *_objp = resolved && ok ? aScopeObj : nsnull;
   return NS_OK;
 }
 

storage/src/mozStorageStatementParams.cpp

@@ -194,15 +194,19 @@ StatementParams::NewResolve(nsIXPConnectWrappedNative *aWrapper,
   // because we want to allow the prototype chain to be checked for the
   // property.
 
-  PRUint32 idx;
-
+  bool resolved = false;
+  PRBool ok = PR_TRUE;
   if (JSVAL_IS_INT(aId)) {
-    idx = JSVAL_TO_INT(aId);
+    PRUint32 idx = JSVAL_TO_INT(aId);
 
     // Ensure that our index is within range.  We do not care about the
     // prototype chain being checked here.
     if (idx >= mParamCount)
       return NS_ERROR_INVALID_ARG;
+
+    ok = ::JS_DefineElement(aCtx, aScopeObj, idx, JSVAL_VOID, nsnull,
+                            nsnull, 0);
+    resolved = true;
   }
   else if (JSVAL_IS_STRING(aId)) {
     JSString *str = JSVAL_TO_STRING(aId);
@@ -213,25 +217,17 @@ StatementParams::NewResolve(nsIXPConnectWrappedNative *aWrapper,
     // the rest of the prototype chain be checked.
     NS_ConvertUTF16toUTF8 name(reinterpret_cast<const PRUnichar *>(nameChars),
                                nameLength);
+    PRUint32 idx;
     nsresult rv = mStatement->GetParameterIndex(name, &idx);
-    if (NS_FAILED(rv)) {
-      *_objp = NULL;
-      return NS_OK;
+    if (NS_SUCCEEDED(rv)) {
+      ok = ::JS_DefineUCProperty(aCtx, aScopeObj, nameChars, nameLength,
+                                 JSVAL_VOID, nsnull, nsnull, 0);
+      resolved = true;
     }
-
-    *_retval = ::JS_DefineUCProperty(aCtx, aScopeObj, nameChars, nameLength,
-                                     JSVAL_VOID, nsnull, nsnull, 0);
-    NS_ENSURE_TRUE(*_retval, NS_OK);
-  }
-  else {
-    // We do not handle other types.
-    return NS_OK;
   }
 
-  *_retval = ::JS_DefineElement(aCtx, aScopeObj, idx, JSVAL_VOID, nsnull,
-                                nsnull, 0);
-  if (*_retval)
-    *_objp = aScopeObj;
+  *_retval = ok;
+  *_objp = resolved && ok ? aScopeObj : nsnull;
   return NS_OK;
 }