120615 - XmlRpc performs HTTP PUT instead of HTTP POST during asynchcall using v 1.80 of nsHttpChannel.cpp

Status: RESOLVED FIXED

Description

[James Warner]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.7+)
Gecko/20020116
BuildID:    0000000000

Problem 1: When performing an asynchCall using the nsIXmlRpcClient interface the
output sent to the receiving server is PUT / HTTP/1.1.  In order for this to
work it needs to be POST instead of PUT (note see additional info below for
temporary fix).

Problem 2:  Setting the content-length in the http header is producing an 
output of: Content-length: 187, 189.  I'm not sure if the Http spec leaves the
Content-length field as a multiple entry field or not, but when trying to use
the Apache XmlRpc implementation for a server this caused the Server to throw a
NumericValue invalid exception.

Reproducible: Always
Steps to Reproduce:
1.  Open up the xml-rpc.xul test file or your favorite XmlRpc test
2.  Start up something that will print out the Http/XML data being sent to that port
3.  Make the XmlRpc call to the server and investigate the HTTP Header info.

Actual Results:  After performing the above steps you should see that in fact
the HTTP Header is displaying a put instead of a POST and you will probably see
that the Content-Length field has a form of: Content-length: int, int.  If on
the other hand you are using an actual XmlRpc Server then the result will most
likely be that you don't get any result Back and the OnError Handler from
XmlRpcClient Listener is called.

Expected Results:  Instead, the HTTP header should have a POST in the header and
the result should be processed correctly... hopefully getting data back to your
application.

Ok, I admit that the first problem at least and perhaps the second problem are
probably more of an issue with the newly modified nsIHttpChannel.cpp code, but
in the interest of finding the quickest WorkAround to the XmlRpc Interface not
working I opted to change the nsXmlRpcClient.js file.  The following hopefully
describes the appropriate changes (sorry no diffs currently).
In function _getChannel find the following line:
chann.setRequestHeader('content-length', request.length);
Remove it.  Probably a questionable idea, but evidently it still figures out the
appropriate number of lines (given my rudimentary test).

Also in function _getChannel after the call to 
postStream.setData(request, request.length);
Place the following line
	chann.requestMethod = 'POST';
This will reset the requestMethod to POST, after it has been overwritten by the
call to postStream.setData so that the information
shows up correctly in the Http header on send.

Comment 1

[Christopher Aillon (sabbatical, not receiving bugmail)]

*** Bug 120732 has been marked as a duplicate of this bug. ***

Comment 2

[Darin Fisher]

the default for nsIUploadChannel::SetUploadStream() changed recently to whatever
the equivalent of "publish" is for a particular protocol.  this is necessary to
support generalized publishing of documents to any given protocol that
implements nsIUploadChannel.

my appologies for not checking the callers more carefully when this change landed!

Comment 3

[Mike Lee]

Created attachment 65727 [details]
xmlrpc

Comment 4

[Mike Lee]

Ok easy fix
I don't know how to create a patch (doh!), so I attach the whole file instead
All it is change:
  upload.setUploadStream(postStream, 'text/xml', -1);
to:
  upload.setUploadStream(postStream, null, -1);

How do I create a patch? :p

Comment 5

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Created attachment 65805 [details] [diff] [review]
Patch based on comments

Comment 6

[Bradley Baetz (:bbaetz)]

Comment on attachment 65805 [details] [diff] [review]
Patch based on comments

r=bbaetz, after it was explained to me ;)

Comment 7

[Darin Fisher]

sr=darin

Comment 8

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

Let's change the status to new... So anyone wanna explain why the patch fixes
this bug, and if it is likely to stay good in the future this way?

I can check this in as soon as the tree opens unless someone beats me to it.

Mike, you need a program called 'diff' to make a diff. If you have downloaded
the Mozilla source via CVS, you can do a 'cvs diff' which is even better than
regular diff. With CVS diff you would do something like:

prompt>cvs diff -u nsXmlRpcClient.js > mychanges.diff

Comment 9

[Mike Lee]

Yeah, because "and pending bug 112479, we have to set the headers manually"
[comment in nsXmlRpcClient.js]. The bug was fixed, header is now set by
nsIHttpChannel::setRequestHeader(). nsIUploadChannel::SetUploadStream() default
PUT when header is set manually. Hopefully I didnt misunderstood anything :)

Thanks Heikki, thought I don't really think its a good idea to pull cvs tree on
a 56k modem :(

Comment 10

[Axel Hecht]

patch coming up. I fixed both the headers and the POST method.
This way, I get the xml-rpc.xul to actually work again. The trick I used was
just setting the method after setUploadStream, if that has to guess the method.

Comment 11

[Axel Hecht]

Created attachment 66094 [details] [diff] [review]
another patch, clean up request header, set request method late enough

Comment 12

[Bradley Baetz (:bbaetz)]

Comment on attachment 66094 [details] [diff] [review]
another patch, clean up request header, set request method late enough

>+        // Set the request method. setUploadStream guesses the method,
>+        // so we gotta do this afterwards.
>+	chann.requestMethod = 'POST';

The last line there is a tab. Fix that, and r=bbaetz

Comment 13

[Darin Fisher]

Comment on attachment 66094 [details] [diff] [review]
another patch, clean up request header, set request method late enough

sr=darin with the indentation fix mentioned by bbaetz

Comment 14

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

Checked in the patch from Axel. Thanks everyone!