Hey, Notes inline
On Mon, Mar 9, 2009 at 3:31 PM, Vincent Siveton <[email protected]>wrote: > Hi Louis, > > 2009/3/9 <[email protected]>: > > Author: lryan > > Date: Mon Mar 9 18:09:06 2009 > > New Revision: 751782 > > > > URL: http://svn.apache.org/viewvc?rev=751782&view=rev > > Log: > > Move image reading to optimizing class associated with image type. > > Clarify failure mode for reading ICC profiles from JPEG's. Dont rely on > Sanselan brittle exception for this case. > > [SNIP] > > > + public static BufferedImage readJpeg(InputStream is) > > + throws ImageReadException, IOException { > > + byte[] bytes = IOUtils.toByteArray(is); > > + // We cant use Sanselan to read JPEG but we can use it to read all > the metadata which is > > + // where most security issues reside anyway in ImageIO > > + Sanselan.getMetadata(bytes, null); > > + byte[] iccBytes = Sanselan.getICCProfileBytes(bytes); > > + if (iccBytes != null && iccBytes.length > 0) { > > + ICC_Profile iccProfile = Sanselan.getICCProfile(bytes, null); > > + if (iccProfile == null) { > > + throw new ImageReadException("Image has ICC but it is corrupt > and cannot be read"); > > + } > > + } > > + return ImageIO.read(new ByteArrayInputStream(bytes)); > > + } > > + > > The call Sanselan.getICCProfile(bytes, null); could display a > stackstrace when processing ByteSourceArray#getBlock() (see > SHINDIG-906) and then could throw a NPE in > Sanselan.getICCProfile(bytes, null); due to a null > org.apache.sanselan.icc.IccProfileInfo. Yes, but this is pretty brittle. It seems safer to defensively code the situation where the image is known to contain icc bytes but they cannot be parsed. The fact that the sanselan code happens to fail with an NPE in this case isnt something we should rely on unless it becomes an explicit contract of their API. > > > Also, a cosmetic thing: any reasons to use null parameter, and not > directly Sanselan.getMetadata(bytes) or Sanselan.getICCProfile(bytes) Nope, typo on my part. > > ? > > Cheers, > > Vincent >
