193953 - WSDL Proxy callback doesn't allow access to the SOAP Response

Status: RESOLVED FIXED

(Core Graveyard :: Web Services, defect, P2)

Description

[Doron Rosenberg (IBM)]

In my wsdl-proxy using sample, I have:

translateCallback : function (aTranslatedValue, aSOAPXPCOMClasses){
  responseSOAP = aSOAPXPCOMClasses.soapResponse.message;

  // serialize message
  .....
}

Trying to serialize the response message gives me a security error. 
aSOAPXPCOMClasses seems to be a nsIWebServiceProxy.

So either we should not expose the message (which is bad, I think its usable for
debugging) or allow accessing of the message.

Comment 1

[Doron Rosenberg (IBM)]

http://lxr.mozilla.org/seamonkey/source/extensions/xmlextras/proxy/public/nsIWebServiceProxy.idl

283 [scriptable, uuid(1ef83ece-b645-4b55-a501-df42c3333b47)]
284 interface nsIWebServiceSOAPCallContext : nsIWebServiceCallContext {
285   /**
286    * For users who want access to the lower-level constructs that
287    * are used for the method invocation, this attributes provides
288    * the SOAP response once the call has completed.
289    *
290    * @see nsISOAPResponse
291    */
292   readonly attribute nsISOAPResponse soapResponse;
293 };

is what I used to be more exact.

Comment 2

[Doron Rosenberg (IBM)]

harish - this still happens - try my wsdl tool and check the checkbox after a
wsdl call to get the error.

Comment 3

[harishd]

Attachment: Patch v1.0 [ not final ]

This patch should fix the problem serializing the response. However, the
response document's properties are still not accessible because the script
security manager denies access if the codebase is changed. Talked to Mitch and
he promised to take a look into it.

Comment 4

[Mitchell Stoltz (not reading bugmail)]

I'm looking at ways to make this cleaner.

Comment 5

[Mitchell Stoltz (not reading bugmail)]

Attachment: Patch part 2

Apply this patch in addition to the one you've got, and things should work.
This patch doesn't change anything in XMLExtras. I realized it was semantically
incorrect to assume that calls to nsAggregatePrincipal::SetCodebase only come
from nsHTMLDocument::SetDomain (obviously, since you're calling SetCodebase as
well), so I added an extra method to nsAggregatePrincipal which SetDomain calls
explicitly to activate the "both pages must set domain" rule. That way, when
XMLRequest calls SetDomain, that restriction will not get in your way.

Regarding your patch, please change the name of ForgePrincipal to something
like ChangePrincipal. ForgePrincipal makes it sound like something
inappropriate, but I think it's perfectly legit - the script is "adopting" the
XML page, just as it would an external .js or CSS file.

Comment 6

[harishd]

Comment on attachment 124478 [details] [diff] [review]
Patch part 2

Nit pick: PRBool, in nsAggregatePrincipal.h, can be replaced with PRPackedBool.

Comment 7

[harishd]

Attachment: Patch v1.2 [ includes patchv1.0 & patch part2 ]

Comment 8

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

Comment on attachment 124492 [details] [diff] [review]
Patch v1.2 [ includes patchv1.0 & patch part2 ]

>Index: extensions/webservices/soap/src/Makefile.in
>===================================================================
>+ 		  widget \

Where does the widget dependency come from?

>Index: extensions/webservices/soap/src/nsHTTPSOAPTransport.cpp
>===================================================================
>+  * codebase pricipal with the subject codebase to.
>+  * override cross domain checks. So use caution 

What's the period doing there?

>+  rv = secMgr->CheckSameOrigin(0, targetURI);

s/0/nsnull/

>+  // forge the principal only if the script security 

s/forge/change/

>         rv = mResponse->SetMessage(document);
>+        ChangePrincipal(document);

Should you change principal before setting the message?

>Index: caps/idl/nsIAggregatePrincipal.idl
>===================================================================
>+    void domainWasSet();
>+    boolean wasDomainSet();

This seems to beg for an attribute instead. How about

attribute boolean domainChanged

>Index: caps/include/nsAggregatePrincipal.h
>===================================================================
>+    PRBool mDomainWasSet;

If you change the IDL to my suggestion then this should be mDomainChanged. Same
with later code.

With those, r=heikki

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 124492 [details] [diff] [review]
Patch v1.2 [ includes patchv1.0 & patch part2 ]

sr=jst with the heikki's changes made.

Comment 10

[harishd]

Attachment: Patch v1.3 [ includes heikki's suggestion ]

Heikki, Could you please rs this patch?

Comment 11

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

Comment on attachment 125429 [details] [diff] [review]
Patch v1.3 [ includes heikki's suggestion ]

rs=heikki for the changes

Comment 12

[harishd]

Fix landed on the trunk.