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

[prasanta sadhukhan]

Thanks Sergey.
Can I get a +1 on this? Phil, are you ok with this?

Regards
Prasanta
On 12/7/2015 9:33 PM, Sergey Bylokhov wrote:
> Thanks for clarification. The fix looks fine then.
>
> On 07.12.15 18:43, prasanta sadhukhan wrote:
>> 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
>>>>
>>>> ||||
>>>
>>>
>>
>
>