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

RESOLVED FIXED in mozilla0.9.9

Status

()

P3
normal
RESOLVED FIXED
17 years ago
16 years ago

People

(Reporter: frodo.baggins5, Assigned: hjtoi-bugzilla)

Tracking

Trunk
mozilla0.9.9
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixinhand])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

17 years ago
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.
*** Bug 120732 has been marked as a duplicate of this bug. ***

Comment 2

17 years ago
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

17 years ago
Created attachment 65727 [details]
xmlrpc

Comment 4

17 years ago
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
Created attachment 65805 [details] [diff] [review]
Patch based on comments
Attachment #65727 - Attachment is obsolete: true
Comment on attachment 65805 [details] [diff] [review]
Patch based on comments

r=bbaetz, after it was explained to me ;)
Attachment #65805 - Flags: review+

Updated

17 years ago
Blocks: 121135

Comment 7

17 years ago
sr=darin
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
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Whiteboard: [fixinhand]
Target Milestone: --- → mozilla0.9.9

Comment 9

17 years ago
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

17 years ago
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

17 years ago
Created attachment 66094 [details] [diff] [review]
another patch, clean up request header, set request method late enough
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
Attachment #66094 - Flags: review+

Comment 13

17 years ago
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
Attachment #66094 - Flags: superreview+
Checked in the patch from Axel. Thanks everyone!
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Updated

16 years ago
QA Contact: petersen → rakeshmishra
You need to log in before you can comment on or make changes to this bug.