Bug 12830 - CSS tokenizer calls isalpha/isalnum with non-ASCII characters
Summary: CSS tokenizer calls isalpha/isalnum with non-ASCII characters
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 420+
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-02-20 11:34 PST by Andrew Dyson
Modified: 2007-02-27 20:59 PST (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Dyson 2007-02-20 11:34:30 PST
In CSSPrimitiveValue.cpp, IsCSSTokenizerIdentifier calls isalpha and isalnum for characters beyond the ASCII range. On Windows, a debug build crashes when I load a page with non-ASCII characters in its CSS [1] because an assertion is fired by isalpha/isalnum (in MS C library) coming across an non-ASCII character. 

Thanks to the way a valid character for CSS identifier is defined in CSS spec, we can neatly avoid this problem just reversing two conditions in two if-statements in the function.

     // {nmstart}
-    if (p == end || !(p[0] == '_' || isalpha(p[0]) || p[0] >= 128))
+    if (p == end || !(p[0] == '_' || p[0] >= 128u || isalpha(p[0])))
         return false;
     ++p;
 
     // {nmchar}*
     for (; p != end; ++p) {
-        if (!(p[0] == '_' || p[0] == '-' || isalnum(p[0]) || p[0] >= 128))
+        if (!(p[0] == '_' || p[0] == '-' || p[0] >= 128u || isalnum(p[0])))


I'll upload a patch soon

[1] 
e.g. Pages like http://www.hani.co.kr  have font names in Korean Hangul in their stylesheets. A lot of CJK pages have font names specified in CJK characters.
Comment 1 Andrew Dyson 2007-02-20 13:57:17 PST
I've realized that this issue was  resolved on 2007-02-06. Sorry for making noise. 
There was no bug associated with the patch so that I can't dupe this bug to that and I'm just resolving it as 'invalid' (not being able to come up with a better resolution)
Comment 2 David Kilzer (:ddkilzer) 2007-02-20 14:14:45 PST
(In reply to comment #1)
> There was no bug associated with the patch so that I can't dupe this bug to
> that and I'm just resolving it as 'invalid' (not being able to come up with a
> better resolution)

It would be helpful to have the Subversion revision number listed here, then I'd reopen/reclose as RESOLVED/FIXED.

Comment 3 Andrew Dyson 2007-02-27 20:58:18 PST
reopening to resolve/fixed
Comment 4 Andrew Dyson 2007-02-27 20:59:21 PST
this was fixed by r19450 | andersca | 2007-02-06 16:28:56 -0800 (Tue, 06 Feb 2007)