[OpenJDK 2D-Dev] <AWT Dev> [rfc]Stream doesn't reset mark in finally

[Jiri Vanek]

On 11/25/2015 06:53 PM, Andrew Hughes wrote:
> ----- Original Message -----
>> On 11/18/2015 06:17 PM, Jiri Vanek wrote:
>>> On 11/12/2015 02:24 PM, Sergey Bylokhov wrote:
>>>> Hi, Jiri.
>>>> This is a valid point, did you file a new CR for this issue?
>>
>> ping?
>>
>>>>
>>>
>>> Hello!
>>>
>>> here it is:
>>> https://jvanek.fedorapeople.org/oracle/jdk8/webrevs/resetInTryFinally/v1/
>>>
>>> Patch:
>>> https://jvanek.fedorapeople.org/oracle/jdk8/webrevs/resetInTryFinally/v1/webrev/
>>> and reprodcuer
>>> https://jvanek.fedorapeople.org/oracle/jdk8/webrevs/resetInTryFinally/v1/MarkTryFinallyReproducer.java
>>>
>>>
>>> Reproducer can be easily turned to jtreg if wonted.
>>>
>>> The patch is for 8, but is valid also for 9, except the path to file.
>>> I will adapt it to 9 once you are ok with it (or you may on your own,
>>> depends on you)
>>>
>>> J.
>>>
>>>
>>>> On 09.11.15 15:45, Alexander Scherbatiy wrote:
>>>>> On 11/7/2015 11:38 AM, Jiri Vanek wrote:
>>>>>> Hello!
>>>>>>
>>>>>> Looking to imageIO.java (if this is bad thread, please redirect me!)
>>>>>
>>>>>       2d-dev alias should be also the right place to ask image related
>>>>> questions in AWT.
>>>>>
>>>>>      Thanks,
>>>>>      Alexandr.
>>>>>
>>>>>> when reading images
>>>>>> http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/javax/imageio/ImageIO.java#l543
>>>>>>
>>>>>>
>>>>>>
>>>>>> and ending at:
>>>>>>
>>>>>>                  // Perform mark/reset as a defensive measure
>>>>>>                  // even though plug-ins are supposed to take
>>>>>>                  // care of it.
>>>>>>                  boolean canDecode = false;
>>>>>>                  if (stream != null) {
>>>>>>                      stream.mark();
>>>>>>                  }
>>>>>>                  canDecode = spi.canDecodeInput(input);
>>>>>>                  if (stream != null) {
>>>>>>                      stream.reset();
>>>>>>                  }
>>>>>>                  return canDecode;
>>>>>>
>>>>>> I'm wondering, why  stream.reset(); is not in finaly  block:
>>>>>>
>>>>>> // Perform mark/reset as a defensive measure
>>>>>> // even though plug-ins are supposed to take
>>>>>> // care of it.
>>>>>> boolean canDecode = false;
>>>>>> if (stream != null) {
>>>>>> stream.mark();
>>>>>> }
>>>>>> try{
>>>>>>      canDecode = spi.canDecodeInput(input);
>>>>>> } finally {
>>>>>>      if (stream != null) {
>>>>>>          stream.reset();
>>>>>>      }
>>>>>> }
>>>>>> return canDecode;
>>>>>>
>>>>>>
>>>>>> Eg png and bmp decoders can are throwing IIOException when header is
>>>>>> corrutped. That pretty definitely sure killer of  stream mark stacks.
>>>>>> You yourselves write  : //Perform mark/reset as a defensive measure
>>>>>> even though plug-ins are supposed to take care of it.
>>>>>> So if densive, then try/finaly please.
>>>>>>
>>>>>> I did not check if this changed in 9 but if not....Please. This is
>>>>>> makig work on custom image plugins, for image formats which just wraps
>>>>>> another bmp/png and are expecting corrupted headers preatty hards.
>>>>>>
>>>>>>
>>>>>> J.
>>>>>
>>>>
>>>>
>>>
>>
>>
>
> I've filed https://bugs.openjdk.java.net/browse/JDK-8144071 for this.

Here you go!

https://jvanek.fedorapeople.org/oracle/jdk9/webrevs/resetInTryFinally/v2/
>
> The change looks good to me. I can confirm the reproducer fails
> on OpenJDK 6, 7 and 8:
>
> Exception in thread "main" java.lang.RuntimeException: exeption from call to canDecodeInput in ImageIO corrupted stack in ImageInputStream
> 	at MarkTryFinallyReproducer.main(MarkTryFinallyReproducer.java:317)
>
> I suggest the test case is converted to jtreg and the patch to
done

> OpenJDK 9 for a new webrev. Once that is posted, I can commit

done
> the change.
>

TYVM!


I run all jtregs, and looked ok.
  J.