282245 - SOAPCall encodes SOAPParameter with floating point number value with comma (,) instead of point (.)

Status: VERIFIED FIXED

(Core Graveyard :: Web Services, defect)

Description

[Martin Honnen]

With Mozilla 1.8 trunk builds (just tested with latest nightly Mozilla 1.8b
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050214 but also
seen one some earlier nightlies) I find that SOAPCall encodes floating point
number values the wrong way, instead of the point (.) a comma (,) is used to
separate the decimals.

This does not happen with 1.7 releases (tested with Mozilla/5.0 (Windows; U;
Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0).

I am testing on a German edition of Windows XP but of course in SOAP the
encoding of a number should not in any way be influenced by the locale settings,
the XSD data type representation for doubles always uses a '.' as the separator.

Test case will be uploaded next, it uses JavaScript to create one SOAPParameter
with a floating point number value, then creates a SOAPCall, calls encode on it
and then outputs the serialization of the SOAP message.

With the Mozilla 1.8 nightly the result is e.g.
  <number-input xsi:type="xs:double">2,500000</number-input>
for the JavaScript number value 2.5 passed in while Mozilla 1.7 releases
correctly encode as
  <number-input xsi:type="xs:double">2.500000</number-input>

Comment 1

[Martin Honnen]

Attachment: test case (serialization of SOAP message is output, check content of <number-input xsi:type="xs:double"> elements for comma instead of point

Comment 2

[Martin Honnen]

I am just trying some older builds to try to find when the bug was introduced:

Bug does not occur with Mozilla 1.8a Mozilla/5.0 (Windows; U; Windows NT 5.1;
en-US; rv:1.8a) Gecko/20040430

Bug does occur with Mozilla 1.8a2 Mozilla/5.0 (Windows; U; Windows NT 5.1;
en-US; rv:1.8a2) Gecko/20040705

Comment 3

[Martin Honnen]

I have tried to look into the C++ code to understand where the comma might come
from respectively where the floating point number value is converted to a string
and I think it is done in
<http://lxr.mozilla.org/mozilla/source/extensions/webservices/soap/src/nsDefaultSOAPEncoder.cpp#992>
the value is originally an nsIVariant and the method GetAsAString is called to
make it into a string.

Then I have looked which other components use nsIVariant and
<http://lxr.mozilla.org/mozilla/ident?i=nsIVariant>
shows that the send method of XMLHttpRequest also takes a variant. Indeed a test
case
<http://www34.brinkster.com/libertydevelop/mozillaBugs/soap/floatingPointXMLHttp.html>
using script and XMLHttpRequest to send a floating point JavaScript number value
to an ASP script that simply echoes back what it receives shows the same problem
as the SOAP encoding, floating point number values such as e.g. 2.5 are
converted to the string e.g. 2,500000 with a comma (,) instead of a dot (.) to
separate the decimals in Mozilla 1.8 nightlies, at least on my machine here with
 the German edition of Windows XP. In Germany floating point numbers indeed use
the comma as the separator so it could be that GetAsString has somewhere been
changed to look at the locale settings when converting floating point numbers.
But that is just a guess.

Boris, do you have an idea what is happening here, why the floating point
numbers are converted to a string with a comma as the decimal separator in
Mozilla 1.8 nightlies for more than six months it seems? For a SOAP parameter
that is certainly plain wrong, I don't know whether other uses of nsIVariant
needed that change.

Comment 4

[David Bradley]

I would imagine that PR_smprintf is doing the locale stuff. That's basically
what the variant is doing when you convert to string.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Yes, absolutely.  Using PR_smprintf for locale-insensitive stuff is wrong,
wrong, wrong.  See
http://lxr.mozilla.org/seamonkey/source/xpcom/string/src/nsStringObsolete.cpp#1300

Is there any reason this code isn't just using AppendFloat/AppendInt on an
nsC?String, anyway?

Note that the code has been this way for nsIVariant since 2001.  So either SOAP
started using nsIVariant recently, or something.

nsIVariant continues to be severely underdefined as to what it returns; jst,
dbaron do you think anything would break if I weaned it off PR_smprintf?

Comment 6

[David Bradley]

I don't see any reason not to use the Append[Float|Int] functions. 

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

I didn't use AppendInt for the ints because it doesn't really do unsigned
stuff.	In any case, that method just calls PR_snprintf, so we should be OK
there.

Comment 8

[Peter Van der Beken [:peterv]]

Note that there's a bunch of PR_smprintf's in nsDefaultSOAPEncoder.cpp too.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix default encoder too

Comment 10

[Peter Van der Beken [:peterv]]

Comment on attachment 174498 [details] [diff] [review]
Patch

Though you don't really need to cast. Either way, r=peterv.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 12

[Martin Honnen]

Verifying fixed with Mozilla 1.8b2 (Mozilla/5.0 (Windows; U; Windows NT 5.1;
en-US; rv:1.8b2) Gecko/20050226)