[OpenJDK 2D-Dev] [9] RFR JDK-7160052: GlyphVector.setGlyphPosition can throw an exception on valid input

[prasanta sadhukhan]

Hi Sergey,

clearCache(index) is called from 2 places one from setGlyphPositions() 
which I fixed and other one from setGlyphTransform() and 
setGlyphTransform is called from public setGlyphTransform() which 
already have a check for IOB
if (ix < 0 || ix >= glyphs.length) {
             throw new IndexOutOfBoundsException("ix = " + ix);
         }
so this (setGlyphPositions) is the only place where it was called wrongly.

Regards
Prasanta
On 12/7/2015 9:01 PM, Sergey Bylokhov wrote:
> Hi, Prasanta.
> Can you please confirm that we properly use clearCaches() in other 
> places of StandardGlyphVector.java?
>
> On 27.11.15 14:48, prasanta sadhukhan wrote:
>> Hi All,
>>
>> Please review a fix for jdk9
>> Bug: https://bugs.openjdk.java.net/browse/JDK-7160052
>> webrev: http://cr.openjdk.java.net/~psadhukhan/7160052/webrev.00/
>>
>> Issue:
>> GlyphVector.setGlyphPosition(int glyphIndex, Point2D
>> <imap://Sergey%2EBylokhov%40oracle%2Ecom@stbeehive.oracle.com:993/fetch%3EUID%3E/mailing%20list/2d%3E5454?header=quotebody&part=1.2.2&filename=Point2D.html> 
>> newPos)
>> can have glyphIndex "equal" to the number of glyphs in this GlyphVector
>> so if user tries to call GlyphVector.setGlyphPosition(gv.getNumGlyphs(),
>> gv.getGlyphPosition(gv.getNumGlyphs()) it throws 
>> IndexOutOfBoundsException
>>
>> Cause:
>> StandardGlyphVector maintains a glyph cache for the glyphs stored in the
>> GlyphVector via lbcache = new Shape[glyphs.length];
>>
>> When GlyphVector.setGlyphPosition() is called, it positions the glyph at
>> the specified glyphIndex|||||||||| and tries to clear the mentioned
>> glyphIndex position of cache by calling clearCache(glyphIndex)
>> But, if we pass the glyphIndex == number of glyphs, then it tries to
>> access beyond the cache array resulting in IOB.
>>
>> Fix:
>> Checked if glyphIndex passed as argument is equal to number of glyphs
>> then do not try to clear the cache as setGlyphPosition() will anyway
>> sets the position after the last glyph.
>> Also, added this check as per spec|IndexOutOfBoundsException
>> <imap://Sergey%2EBylokhov%40oracle%2Ecom@stbeehive.oracle.com:993/fetch%3EUID%3E/mailing%20list/2d%3E5454?header=quotebody&part=1.2.3&filename=IndexOutOfBoundsException.html>| 
>>
>> - if |glyphIndex| is less than 0 or greater than the number of glyphs in
>> this |GlyphVector
>> |
>> Regards
>> Prasanta
>>
>> ||||
>
>