Thanks Louis! Cheers,
Vincent 2009/3/9 Louis Ryan <[email protected]>: > 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 >> >
