[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] <AWT Dev> [rfc]Stream doesn't reset mark in finally
From:       Jiri Vanek <jvanek () redhat ! com>
Date:       2015-11-27 12:57:20
Message-ID: 56585330.7050500 () redhat ! com
[Download RAW message or body]

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.


[prev in list] [next in list] [prev in thread] [next in thread]