XML-RPC bug handling dates with months 10 and over

RESOLVED FIXED

Status

()

Core
XML
RESOLVED FIXED
16 years ago
16 years ago

People

(Reporter: C. Daniel Mojoli B., Assigned: Samuel Sieb)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

16 years ago
Small bug in nsXmlRpcClient.js implementation. The + operator adds numerically
instead of concatenating. The patch is very simple.
(Reporter)

Comment 1

16 years ago
Created attachment 78954 [details] [diff] [review]
Fix for small bug in nsXmlRpcClient.js script. Solves date problem.
(Reporter)

Comment 2

16 years ago
Created attachment 78957 [details] [diff] [review]
The correct patch for the Date problem in nsXmlRpcClient.js script

I got the original patch backwards. . .
Reassigning to XML-RPC owner.
Assignee: heikki → samuel
(Reporter)

Comment 4

16 years ago
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.
(Assignee)

Comment 5

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

r=ssieb
Attachment #78957 - Flags: review+
(Assignee)

Comment 6

16 years ago
looking for sr=
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: patch

Comment 7

16 years ago
Changing QA Contact
QA Contact: petersen → rakeshmishra
(Assignee)

Comment 8

16 years ago
Created attachment 80059 [details] [diff] [review]
new patch
(Assignee)

Updated

16 years ago
Attachment #78954 - Attachment is obsolete: true
Attachment #78957 - Attachment is obsolete: true
(Assignee)

Comment 9

16 years ago
Comment on attachment 80059 [details] [diff] [review]
new patch

r=rginda (from irc)
Attachment #80059 - Flags: review+
(Assignee)

Comment 10

16 years ago
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.
looks like the "day" code a lone below has the same problem.

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

Comment 12

16 years ago
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 on attachment 80059 [details] [diff] [review]
new patch

sr=sspitzer

thanks for the info.
Attachment #80059 - Flags: superreview+

Comment 14

16 years ago
Comment on attachment 80059 [details] [diff] [review]
new patch

a=asa (on behalf of drivers) for checkin to the 1.0 branch
Attachment #80059 - Flags: approval+
(Assignee)

Comment 15

16 years ago
timeless checked it in on the branch
Keywords: fixed1.0.0

Comment 16

16 years ago
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...
(Reporter)

Comment 17

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

16 years ago
Can anyone provide a test case so that this bug could be verified
(Reporter)

Comment 19

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

16 years ago
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
Keywords: verified1.0.0
(Assignee)

Comment 21

16 years ago
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.