Closed Bug 37395 Opened 20 years ago Closed 20 years ago

nsString::IsASCII return true for non ASCII

Categories

(Core :: XPCOM, defect, P3)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: ftang, Assigned: ftang)

Details

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.
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
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.
hum... I have a bug which is depend on this....
There are currently no such function called nsCRT::IsAscii(). I have no way to 
figure out it is right or wrong.
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.
I have fix the nsString::IsASCII and add nsCRT::IsASCII . Mark it fixed.
Assignee: scc → ftang
fix and check in.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
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.