282672 - nsSchemaValidator::IsValidSchemaFloat needs fixed

Status: RESOLVED FIXED

(Core Graveyard :: Web Services, defect)

Description

[Jonathan Watt [:jwatt]]

As noted by bz, PR_snprintf is locale-sensitive. After converting from a string
to a float and back to a string, the resulting string may contain a different
decimal separator from the original. Also the comparison should be
if(strcmp()!=0) and not if(strcmp()==0).

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: perhaps this?

Any reason(s) why we can't just do this?

Comment 2

[Doron Rosenberg (IBM)]

Actually, my new code (being reviewed) does do this :)  But its a good idea to
fix the existing code.

Comment 3

[Doron Rosenberg (IBM)]

Comment on attachment 174654 [details] [diff] [review]
perhaps this?

I'll check it in when I get into work (no need for sr, this is
nont-built-by-default code :)

Comment 4

[Jonathan Watt [:jwatt]]

I can check in my own patches -- unless you have a really strong preference for
doing it yourself. ;-)

I presume you noticed that this patch will allow leading and trailing whitespace
around the value. Also, it seems to me that the way things are currently done
should return false for numbers with more decimal digits than can be stored in a
float.

Where is the code that's being reviewed BTW?

Comment 5

[Jonathan Watt [:jwatt]]

(In reply to comment #4)
> I can check in my own patches

Although I should point out that it was bz that suggested that ToFloat could be
used.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Is there a reason that patch is still converting the string to UTF8?

Comment 7

[Jonathan Watt [:jwatt]]

Hmm, no, good point. But why isn't ToFloat in nsAString?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Because it was only implemented for nsString/nsCString....  But you could use an
nsAutoString here.

Comment 9

[Doron Rosenberg (IBM)]

bug 223097 has the new, nsAutoString using version of the code.  Actually, it
seems I never got around to changing Float to using what you are, thought I had.

Comment 10

[Jonathan Watt [:jwatt]]

Attachment: patch using nsAutoString

Good point. Here's a new patch.

Comment 11

[Jonathan Watt [:jwatt]]

doron: I notice that you took out the "#ifdef DEBUG" code from your new patch.
Do you want that here too?

Comment 12

[Doron Rosenberg (IBM)]

no need

Comment 13

[Jonathan Watt [:jwatt]]

Marking FIXED.

Checking in mozilla/extensions/schema-validation/src/nsSchemaValidator.cpp;
/cvsroot/mozilla/extensions/schema-validation/src/nsSchemaValidator.cpp,v  <--
nsSchemaValidator.cpp
new revision: 1.9; previous revision: 1.8
done