137171 - XML-RPC bug handling dates with months 10 and over

Status: RESOLVED FIXED

(Core :: XML, defect)

Description

[C. Daniel Mojoli B.]

Small bug in nsXmlRpcClient.js implementation. The + operator adds numerically
instead of concatenating. The patch is very simple.

Comment 1

[C. Daniel Mojoli B.]

Attachment: Fix for small bug in nsXmlRpcClient.js script. Solves date problem.

Comment 2

[C. Daniel Mojoli B.]

Attachment: The correct patch for the Date problem in nsXmlRpcClient.js script

I got the original patch backwards. . .

Comment 3

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

Reassigning to XML-RPC owner.

Comment 4

[C. Daniel Mojoli B.]

I was too terse in describing this minor yet troublesome bug. Perhaps this
clarification will attract more attention.

In function iso8601Format(date) of nsXmlRpcClient.js, the local variable
"datetime" is first assigned a number (the year). The month must be appended to
that with a "0" prefix if the month is lesser than 10. For single digit months
the *string* '0' is added to the original datetime variable turning it into a
string. Further uses of the '+' operator will then concatenate properly.

For double digit months, the '0' string is NOT appended to datetime so it
remains a number. When the '+' operator is used to concatenate datetime and
month it incorrectly *ADDS* the year and the two digit month!

I use XML-RPC extensively as part of a larger protocol and this change is *so*
trivial to implement I suggest it should form part of Mozilla 1.0 candidate even
though the cut has been made.

Comment 5

[Samuel Sieb]

Comment on attachment 78957 [details] [diff] [review]
The correct patch for the Date problem in nsXmlRpcClient.js script

r=ssieb

Comment 6

[Samuel Sieb]

looking for sr=

Comment 7

[Chris Petersen]

Changing QA Contact

Comment 8

[Samuel Sieb]

Attachment: new patch

Comment 9

[Samuel Sieb]

Comment on attachment 80059 [details] [diff] [review]
new patch

r=rginda (from irc)

Comment 10

[Samuel Sieb]

Looking for an sr= please.  If no one on the cc list can, then please let me
know who can.  It's a very simple JS fix.

Comment 11

[(not reading, please use [e-mail] instead)]

looks like the "day" code a lone below has the same problem.

var day = date.getUTCDate();
datetime += (day < 10 ? '0' + day : day);

Comment 12

[Samuel Sieb]

I already had thought of that.  |datetime| is guaranteed to be a string, so
there's no problem adding an integer because the integer will be converted to a
string and appended.

Comment 13

[(not reading, please use [e-mail] instead)]

Comment on attachment 80059 [details] [diff] [review]
new patch

sr=sspitzer

thanks for the info.

Comment 14

[Asa Dotzler [:asa]]

Comment on attachment 80059 [details] [diff] [review]
new patch

a=asa (on behalf of drivers) for checkin to the 1.0 branch

Comment 15

[Samuel Sieb]

timeless checked it in on the branch

Comment 16

[Giacomo Magnini]

After this patch was checked on the branch, bloat test were up by ~600KB. The
only other checkin occurred in the same time frame was a Mac only one (bug
141330), while the stats were seen on a linux box. Maybe this will need some
more investigation...

Comment 17

[C. Daniel Mojoli B.]

This patch can't be accountable for any bloat. It is way too simple as the only
problem was a missing cast. No logic was added or ammended. What you mention is
strange, but I wouldn't start looking here.

Comment 18

[Rakesh Mishra]

Can anyone provide a test case so that this bug could be verified

Comment 19

[C. Daniel Mojoli B.]

I reported this bug originally. I have no problems with it as the patched
applied for RC2 (or RC1, can't remember) solved it.

My application is too large and interdependent (read as: messy) to post here but
the code following should suffice. Point it to some webserver and tcpdump the
POST request. You don't need to set up anything on the other side because the
problem was the request body. You will see that the date format now adheres
properly to ISO 8601, as opposed to loosing required digits in the date format
because digits were added instead of concatenated.

----
var xmlRpc=Components.classes['@mozilla.org/xml-rpc/client;1']
           .createInstance(Components.interfaces.nsIXmlRpcClient);
xmlRpc.init("http://myserver.somewhereinthe.net/xmlrpc");
var someDate=xmlRpc.createType(xmlRpc.DATETIME, {});
someDate.data=new Date("2002/10/31");

var listener = { /* ignoring everything */ };
xmlRpc.asyncCall(listener, null, 'someHandler.someMethod', [ someDate ], 1);
----

Comment 20

[Rakesh Mishra]

Verifying on the branch build 2002-05-31-08-1.0.0 , as per the patch and the
comments in the Comment #19
Adding "verified1.0.0" to the keyword

Comment 21

[Samuel Sieb]

checked in