nsString::IsASCII return true for non ASCII

RESOLVED FIXED

Status

()

Core
XPCOM
P3
normal
RESOLVED FIXED
18 years ago
18 years ago

People

(Reporter: Frank Tang, Assigned: Frank Tang)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

18 years ago
The implementation of nsString::IsASCII is wrong. The ASCII range is only from
U+0000 to U+007F. However, the current implementation define it as
U+0000 to U+00FF. We should change the implementation so it will return
false from U+0080 to U+00FF, even for the eOneByte case

2071 PRBool nsString::IsASCII(const PRUnichar* aBuffer) {
2072
2073 if(!aBuffer) {
2074 aBuffer=mUStr;
2075 if(eOneByte==mCharSize)
2076 return PR_TRUE;
2077 }
2078 if(aBuffer) {
2079 while(*aBuffer) {
2080 if(*aBuffer>255){
2081 return PR_FALSE;
2082 }
2083 aBuffer++;
2084 }
2085 }
2086 return PR_TRUE;
2087 }
2088

Fortunately, there are no code call this API yet. So, it should not be a big
deal to fix it now.
(Assignee)

Comment 1

18 years ago
scc, I am very confused who is the owner of nsString2.cpp now. I assign it to 
you. Here is the patch. I can check in for you if you agree with it 

Index: nsString2.cpp
===================================================================
RCS file: /m/pub/mozilla/xpcom/ds/nsString2.cpp,v
retrieving revision 1.89
diff -u -r1.89 nsString2.cpp
--- nsString2.cpp       2000/04/26 00:54:09     1.89
+++ nsString2.cpp       2000/04/27 18:59:24
@@ -2071,13 +2071,22 @@
 PRBool nsString::IsASCII(const PRUnichar* aBuffer) {
 
   if(!aBuffer) {
-    aBuffer=mUStr;
-    if(eOneByte==mCharSize)
+    if(eOneByte==mCharSize) {
+        char* aByte = mStr;
+        while(*aByte) {
+          if(*aByte & 0x80) { // don't use (*aByte > 0x7F) since char is signed
+            return PR_FALSE;        
+          }
+          aByte++;
+        }
       return PR_TRUE;
+    } else {
+      aBuffer=mUStr; // let the following code handle it
+    }
   }
   if(aBuffer) {
     while(*aBuffer) {
-      if(*aBuffer>255){
+      if(*aBuffer>0x007F){
         return PR_FALSE;        
       }
       aBuffer++;

see http://warp/u/ftang/tmp/asciidiff.txt also

Comment 2

18 years ago
Don't bother to fix this; nsString::isAscii() is going away. The new method is 
nsCRT::IsAscii(), which may have the same problem. Please focus your efforts 
there.
(Assignee)

Comment 3

18 years ago
hum... I have a bug which is depend on this....
(Assignee)

Comment 4

18 years ago
There are currently no such function called nsCRT::IsAscii(). I have no way to 
figure out it is right or wrong.

Comment 5

18 years ago
Ok -- let me be more precise: the nsString method is going away, and is replaced 
by nsCRT::IsAsciiAlpha(), nsCRT::IsAsciiDigit(), nsCRT::IsAsciiSpace(). If you 
want to fix the problem, please do it there.
(Assignee)

Comment 6

18 years ago
I have fix the nsString::IsASCII and add nsCRT::IsASCII . Mark it fixed.
Assignee: scc → ftang
(Assignee)

Comment 7

18 years ago
fix and check in.
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 8

18 years ago
assigning qa contact to ftang; Frank, if you give QA some way to verify it they 
can help, otherwise, all yours :-)
QA Contact: leger → ftang
You need to log in before you can comment on or make changes to this bug.