#3 CharToGlyphIndex has no error checking for high unicode vals

closed-fixed
None
5
2010-05-23
2008-06-19
SilverDirk
No

FTCharToGlyphIndex.h implements a sparse array of glyph indicies.

There are two problems with it: first, it doesn't check the character value passed to it, so any character over 0xFFFF will segfault. Second, it is using "div" to generate the bucket indices... which since this isn't a hash table, there is no reason to do this.

The max defined unicode character point is 0x10FFFF (according to wikipedia) so thats 21 bits. Rather than extending the buckets, I decided to convert from a 2-level array to 3-level, with each level having 7 bits.

The slight loss of speed from the extra lookup should be more than made up for by avoiding the div instruction; I used bitshifts instead.

Also, I added 'if' statements that ignore any character request >= 0x11000. This is the most important part, because before, data external to the program would have been able to cause a buffer overrun of up to 0xFFFF00 beyond the end of the 256-element array, which could be quite dangerous.

I also was about to implement a static "sentinel" index, so that instead of pointing to NULL, the empty buckets would point to Sentinel and this would get rid of the 'if' statements in 'find()'. However, its something of a mess to get static vars initialized in a library and I decided it was a bit overkill. (but if you like that idea, I'll go ahead and write it)

Discussion

  • SilverDirk

    SilverDirk - 2008-06-19

    patch to update CharToGlyphIndex to a 3-layer design, and check for invalid chars

     
  • SilverDirk

    SilverDirk - 2008-06-19

    corrected patch to update CharToGlyphIndex to a 3-layer design, and check for invalid chars

     
  • SilverDirk

    SilverDirk - 2008-06-19

    Logged In: YES
    user_id=1014397
    Originator: YES

    Wait! Small bug in there.

    BucketIdxSize= 1<<(BucketIdxBits-1)

    should be

    BucketIdxSize= 1<<BicketIdxBits
    File Added: CharToGlyphIndex_fix.patch

     
  • Lars Huttar

    Lars Huttar - 2008-07-25

    Logged In: YES
    user_id=294943
    Originator: NO

    Thanks for this fix. I applied it to my copy of the code.
    Lars (just a user)

     
  • Sam Hocevar

    Sam Hocevar - 2010-05-23
    • assigned_to: nobody --> sammy
    • status: open --> closed-fixed
     
  • Sam Hocevar

    Sam Hocevar - 2010-05-23

    The segfaults were already fixed, but your patch properly addresses the underlying problem. I do not believe we should worry about performance here. I applied it and reworked it slightly. Thanks.

     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks