193387 - Unable to use BabelFish service

Status: RESOLVED FIXED

(Core :: XML, defect, P2)

Description

[harishd]

When I tried using BabelFish translator service I crashed with the following stack:

nsVariant::ConvertToAString(const nsDiscriminatedUnion & {...}, nsAString &
{...}) line 847
nsVariant::GetAsAString(nsVariant * const 0x0348ce88, nsAString & {...}) line
1788 + 16 bytes
WSPProxy::VariantToValue(unsigned char 15, void * 0x0012f810, nsIInterfaceInfo *
0x00000000, nsIVariant * 0x0348ce88) line 966 + 16 bytes
WSPProxy::VariantToInParameter(nsIInterfaceInfo * 0x033b3d50, unsigned int 4,
const nsXPTParamInfo * 0x0012f7ac, nsIVariant * 0x0348ce88, nsXPTCVariant *
0x0012f810) line 835 + 26 bytes
WSPCallContext::CallCompletionListener() line 367 + 50 bytes
WSPCallContext::HandleResponse(WSPCallContext * const 0x0326e774,
nsISOAPResponse * 0x0326e8ec, nsISOAPCall * 0x0320e5cc, unsigned int 0, int 1, 

This is because of line 961 in wspproxy.cpp (
http://lxr.mozilla.org/seamonkey/source/extensions/xmlextras/proxy/src/wspproxy.cpp#961
). nsVariant::ConvertToAString expects an nsAString and hence instead of passing
in a void* we should pass in a real string.

Comment 1

[Gábor Lipták]

the URL specified above works with

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3b) Gecko/20030214

Comment 2

[harishd]

Gábor Lipták: It's not the URL itself but creating a proxy and using the service
crashes mozilla. May be my comments should have been more explicit.

To reproduce the problem:

1) Set environment variable MOZ_WSP and build mozilla/extensions. Note: You
should also build interfaceinfo under mozilla. 
2) Create a proxy service ( follow the samples in
http://lxr.mozilla.org/seamonkey/source/extensions/xmlextras/proxy/src/tests/ )
3) Try and use BableFishService and you should see the crash.

Comment 3

[harishd]

Fixed along with bug 194349.

Comment 4

[harishd]

For some reason I'm able to reproduce the crash still. Reopening.

Comment 5

[Johnny Stenback (:jst)]

Attachment: Give nsXPTCVariant's that are expected to recieve a T_DOMSTRING value an nsAString object to hold the string value.

This patch makes the code that calls the completion listeners properly deal
with string arguments. The code expected to get the value of a DOMString
nsIVariant into an nsXPTCVariant w/o giving the nsXPTCVariant an nsAString
object to hold the data.

This patch also cleans up some code in wsppropertybagwrapper.cpp (basically
just indentation changes), that cleanup is unrelated to this bug, the real
changes are in wspcallcontext.cpp and wspproxy.cpp.

Comment 6

[harishd]

Comment on attachment 115953 [details] [diff] [review]
Give nsXPTCVariant's that are expected to recieve a T_DOMSTRING value an nsAString object to hold the string value.

>Index: extensions/xmlextras/proxy/src/wspcallcontext.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xmlextras/proxy/src/wspcallcontext.cpp,v
>retrieving revision 1.5
>diff -u -r1.5 wspcallcontext.cpp
>--- extensions/xmlextras/proxy/src/wspcallcontext.cpp	8 Jan 2003 20:15:34 -0000	1.5
>+++ extensions/xmlextras/proxy/src/wspcallcontext.cpp	1 Mar 2003 01:32:49 -0000
>@@ -241,10 +241,15 @@
>     dp->val.p = nsnull;
>   }
>  
>-  PRUint32 headerCount, bodyCount;
>+  PRUint32 headerCount = 0, bodyCount = 0;
>   nsISOAPHeaderBlock** headerBlocks;
>   nsISOAPParameter** bodyBlocks;
>   
>+#define STRING_ARRAY_BUF_SIZE 2
>+  nsAutoString string_array_buf[STRING_ARRAY_BUF_SIZE];
>+  nsAutoString *string_array = &string_array_buf[0];
>+  PRUint32 string_array_index = 0;
>+
>   // If we have an exception, report it now
>   if (mException) {
>     dispatchParams[0].val.p = NS_STATIC_CAST(nsIException*, mException);
>@@ -293,7 +298,7 @@
>     output->GetPartCount(&partCount);
> 
>     PRUint32 maxParamIndex = methodInfo->GetParamCount()-1;
>-    
>+
>     PRUint32 bodyEntry = 0, headerEntry = 0, paramIndex = 0;
>     for (i = 0; i < partCount; paramIndex++, i++) {
>       nsCOMPtr<nsIWSDLPart> part;
>@@ -351,16 +356,40 @@
>         goto call_completion_end;
>       }
> 
>-      nsXPTCVariant* vars = dispatchParams+paramIndex;
>+      nsXPTCVariant* vars = dispatchParams + paramIndex;
>      
>       if (paramIndex < maxParamIndex &&
>           methodInfo->GetParam((PRUint8)(paramIndex+1)).GetType().IsArray()) {
>-        paramIndex++;        
>+        paramIndex++;
>       }
> 
>       NS_ASSERTION(paramIndex <= maxParamIndex, "WSDL/IInfo param count mismatch");
>       
>       const nsXPTParamInfo& paramInfo = methodInfo->GetParam(paramIndex);
>+
>+      if (XPT_TDP_TAG(paramInfo.type.prefix) == TD_DOMSTRING) {
>+        // If there's no more room in the string buffer on the stack
>+        // for this parameter, allocate an array on the heap.
>+        if (string_array != &string_array_buf[0] &&
>+            string_array_index >= STRING_ARRAY_BUF_SIZE) {
>+          string_array = new nsAutoString[partCount - i];
>+
>+          if (!string_array) {
>+            rv = NS_ERROR_OUT_OF_MEMORY;
>+
>+            goto call_completion_end;
>+          }
>+
>+          // Reset string_array_index now that we switched arrays.
>+          string_array_index = 0;
>+        }
>+
>+        // Give the variant value a nsAString object to hold the data
>+        // in.
>+        vars->val.p =
>+          NS_STATIC_CAST(nsAString *, &string_array[string_array_index++]);
>+      }
>+
>       rv = WSPProxy::VariantToInParameter(listenerInterfaceInfo,
>                                           mListenerMethodIndex,
>                                           &paramInfo,
>@@ -393,6 +422,10 @@
>   if(dispatchParams && dispatchParams != paramBuffer) {
>     delete [] dispatchParams;
>   }
>+  if (string_array != &string_array_buf[0]) {
>+    delete [] string_array;
>+  }
>+
>   nsCOMPtr<nsIWebServiceCallContext> kungFuDeathGrip(this);
>   mProxy->CallCompleted(this);
>   NS_RELEASE(mProxy);
>Index: extensions/xmlextras/proxy/src/wspproxy.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xmlextras/proxy/src/wspproxy.cpp,v
>retrieving revision 1.10
>diff -u -r1.10 wspproxy.cpp
>--- extensions/xmlextras/proxy/src/wspproxy.cpp	8 Jan 2003 20:15:37 -0000	1.10
>+++ extensions/xmlextras/proxy/src/wspproxy.cpp	1 Mar 2003 01:32:49 -0000
>@@ -824,16 +824,25 @@
>     return VariantToArrayValue(arrayType.TagPart(), aXPTCVariant,
>                                iinfo, aVariant);
>   }
>-  else {
>-    if (type.IsInterfacePointer()) {
>-      rv = aInterfaceInfo->GetInfoForParam(aMethodIndex, aParamInfo, 
>-                                           getter_AddRefs(iinfo));
>-      if (NS_FAILED(rv)) {
>-        return rv;
>-      }
>+  // else
>+  if (type.IsInterfacePointer()) {
>+    rv = aInterfaceInfo->GetInfoForParam(aMethodIndex, aParamInfo, 
>+                                         getter_AddRefs(iinfo));
>+    if (NS_FAILED(rv)) {
>+      return rv;
>     }
>-    return VariantToValue(type_tag, &aXPTCVariant->val, iinfo, aVariant);
>   }
>+
>+  if (type_tag == nsXPTType::T_DOMSTRING) {
>+    // T_DOMSTRING values are expected to be stored in an nsAString
>+    // object pointed to by the nsXPTCVariant...
>+    return VariantToValue(type_tag, aXPTCVariant->val.p, iinfo, aVariant);
>+  }
>+  // else
>+
>+  // ... but other types are expected to be stored directly in the
>+  // variant itself.
>+  return VariantToValue(type_tag, &aXPTCVariant->val, iinfo, aVariant);
> }
> 
> nsresult 
>Index: extensions/xmlextras/proxy/src/wsppropertybagwrapper.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xmlextras/proxy/src/wsppropertybagwrapper.cpp,v
>retrieving revision 1.4
>diff -u -r1.4 wsppropertybagwrapper.cpp
>--- extensions/xmlextras/proxy/src/wsppropertybagwrapper.cpp	8 Jan 2003 20:15:36 -0000	1.4
>+++ extensions/xmlextras/proxy/src/wsppropertybagwrapper.cpp	1 Mar 2003 01:32:49 -0000
>@@ -39,6 +39,8 @@
>  * ***** END LICENSE BLOCK ***** */
> 
> #include "wspprivate.h"
>+#include "nsIXPConnect.h"
>+#include "jsapi.h"
> 
> 
> WSPPropertyBagWrapper::WSPPropertyBagWrapper()
>@@ -108,85 +110,81 @@
>                                   const nsXPTMethodInfo* info,
>                                   nsXPTCMiniVariant* params)
> {
>-  nsresult rv = NS_OK;
>-  nsAutoString propName;
>-  nsCOMPtr<nsIVariant> val;
>-
>   if (methodIndex < 3) {
>     NS_ERROR("WSPPropertyBagWrapper: bad method index");
>     return NS_ERROR_FAILURE;
>   }
>-  else {
>-    rv = WSPFactory::C2XML(nsDependentCString(info->GetName()),
>-                           propName);
>-    if (NS_FAILED(rv)) {
>-      return rv;
>-    }
>-    
>-    rv = mPropertyBag->GetProperty(propName, getter_AddRefs(val));
>-    if (NS_FAILED(rv)) {
>-      return rv;
>-    }
>-    
>-    nsCOMPtr<nsIInterfaceInfo> iinfo;
>-    if (info->IsGetter()) {
>-      const nsXPTParamInfo& paramInfo = info->GetParam(0);
>-      const nsXPTType& type = paramInfo.GetType();
>-      uint8 type_tag = type.TagPart();
>-      
>-      if (type_tag == nsXPTType::T_INTERFACE) {
>-        rv = mInterfaceInfo->GetInfoForParam(methodIndex, &paramInfo, 
>-                                             getter_AddRefs(iinfo));
>-        if (NS_FAILED(rv)) {
>-          return rv;
>-        }
>-      }
>-      
>-      rv = WSPProxy::VariantToValue(type_tag, params[0].val.p, iinfo, val);
>-    }
>-    else if (info->GetParamCount() == 2) {
>-      // If it's not an explicit getter, it has to be an array 
>-      // getter method.
>-
>-      // The first parameter should be the array length out param
>-      const nsXPTParamInfo& paramInfo1 = info->GetParam(0);
>-      const nsXPTType& type1 = paramInfo1.GetType();
>-      if (!paramInfo1.IsOut() || (type1.TagPart() != nsXPTType::T_U32)) {
>-        NS_ERROR("Unexpected parameter type for getter");
>-        return NS_ERROR_FAILURE;
>-      }
> 
>-      // The second parameter should be the array out pointer
>-      // itself.
>-      const nsXPTParamInfo& paramInfo2 = info->GetParam(1);
>-      const nsXPTType& type2 = paramInfo2.GetType();
>-      if (!paramInfo2.IsOut() || !type2.IsArray()) {
>-        NS_ERROR("Unexpected parameter type for getter");
>-        return NS_ERROR_FAILURE;
>-      }
>+  nsresult rv = NS_OK;
>+  nsAutoString propName;
>+
>+  rv = WSPFactory::C2XML(nsDependentCString(info->GetName()),
>+                         propName);
>+  if (NS_FAILED(rv)) {
>+    return rv;
>+  }
> 
>-      nsXPTType arrayType;
>-      rv = mInterfaceInfo->GetTypeForParam(methodIndex, &paramInfo2,
>-                                           1, &arrayType);
>+  nsCOMPtr<nsIVariant> val;
>+  rv = mPropertyBag->GetProperty(propName, getter_AddRefs(val));
>+  if (NS_FAILED(rv)) {
>+    return rv;
>+  }
>+
>+  nsCOMPtr<nsIInterfaceInfo> iinfo;
>+  if (info->IsGetter()) {
>+    const nsXPTParamInfo& paramInfo = info->GetParam(0);
>+    const nsXPTType& type = paramInfo.GetType();
>+    uint8 type_tag = type.TagPart();
>+
>+    if (type_tag == nsXPTType::T_INTERFACE) {
>+      rv = mInterfaceInfo->GetInfoForParam(methodIndex, &paramInfo, 
>+                                           getter_AddRefs(iinfo));
>       if (NS_FAILED(rv)) {
>         return rv;
>       }
>-    
>-      if (arrayType.IsInterfacePointer()) {
>-        rv = mInterfaceInfo->GetInfoForParam(methodIndex, &paramInfo2, 
>-                                             getter_AddRefs(iinfo));
>-        if (NS_FAILED(rv)) {
>-          return rv;
>-        }
>-      }
>+    }
> 
>-      rv = WSPProxy::VariantToArrayValue(arrayType.TagPart(), params, 
>-                                         iinfo, val);
>+    rv = WSPProxy::VariantToValue(type_tag, params[0].val.p, iinfo, val);
>+  } else if (info->GetParamCount() == 2) {
>+    // If it's not an explicit getter, it has to be an array getter
>+    // method.
>+
>+    // The first parameter should be the array length out param
>+    const nsXPTParamInfo& paramInfo1 = info->GetParam(0);
>+    const nsXPTType& type1 = paramInfo1.GetType();
>+    if (!paramInfo1.IsOut() || (type1.TagPart() != nsXPTType::T_U32)) {
>+      NS_ERROR("Unexpected parameter type for getter");
>+      return NS_ERROR_FAILURE;
>     }
>-    else {
>-      NS_ERROR("Unexpected method signature for property bag wrapper");
>+
>+    // The second parameter should be the array out pointer itself.
>+    const nsXPTParamInfo& paramInfo2 = info->GetParam(1);
>+    const nsXPTType& type2 = paramInfo2.GetType();
>+    if (!paramInfo2.IsOut() || !type2.IsArray()) {
>+      NS_ERROR("Unexpected parameter type for getter");
>       return NS_ERROR_FAILURE;
>     }
>+
>+    nsXPTType arrayType;
>+    rv = mInterfaceInfo->GetTypeForParam(methodIndex, &paramInfo2,
>+                                         1, &arrayType);
>+    if (NS_FAILED(rv)) {
>+      return rv;
>+    }
>+
>+    if (arrayType.IsInterfacePointer()) {
>+      rv = mInterfaceInfo->GetInfoForParam(methodIndex, &paramInfo2, 
>+                                           getter_AddRefs(iinfo));
>+      if (NS_FAILED(rv)) {
>+        return rv;
>+      }
>+    }
>+
>+    rv = WSPProxy::VariantToArrayValue(arrayType.TagPart(), params, 
>+                                       iinfo, val);
>+  } else {
>+    NS_ERROR("Unexpected method signature for property bag wrapper");
>+    return NS_ERROR_FAILURE;
>   }
> 
>   return rv;

Comment 7

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 115953 [details] [diff] [review]
Give nsXPTCVariant's that are expected to recieve a T_DOMSTRING value an nsAString object to hold the string value.

>Index: extensions/xmlextras/proxy/src/wspcallcontext.cpp
>===================================================================

>+  nsAutoString *string_array = &string_array_buf[0];

>+        if (string_array != &string_array_buf[0] &&
>+            string_array_index >= STRING_ARRAY_BUF_SIZE) {
>+          string_array = new nsAutoString[partCount - i];

I am wondering if the if-test should be == instead of !=. From this patch alone
it seems like we initialize string_array to point to &string_array_buf[0], and
the only place where we change string_array is inside the if, but we would
never get there with the test as it is now.

If you can explain/fix that, sr=heikki.

Comment 8

[harishd]

Fix landed.