[jira] [Updated] (IMAGING-164) Simplify code in IcoImageParser::writeImage

["Bruno P. Kinoshita (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/IMAGING-164?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bruno P. Kinoshita updated IMAGING-164:
---------------------------------------
    Summary: Simplify code in  IcoImageParser::writeImage  (was: Possible dereferencing of null pointer in IcoImageParser::writeImage)

> Simplify code in  IcoImageParser::writeImage
> --------------------------------------------
>
>                 Key: IMAGING-164
>                 URL: https://issues.apache.org/jira/browse/IMAGING-164
>             Project: Commons Imaging
>          Issue Type: Improvement
>          Components: Format: ICO
>            Reporter: Michael Groß
>            Priority: Major
>              Labels: github
>             Fix For: Review Patch
>
>
> org.apache.commons.imaging.formats.ico.IcoImageParser::writeImage(final BufferedImage src, final OutputStream os, final ImagingParameters params)
> may throw an unexpected NullPointerException because it of the following code:
> {noformat}
> final SimplePalette palette = paletteFactory.makeExactRgbPaletteSimple(src, 256);
> {noformat}
> Then asking if the created palette is null. I will discuss where it comes from below. For now it is interesting that we set the variable bitCount despite the SimplePalette is null. Currently this makes no sense because the code will throw a NullPointerException later if SimplePalette is null.
> {noformat}
> if (palette == null) {
>             if (hasTransparency) {
>                 bitCount = 32;
>             } else {
>                 bitCount = 24;
>             }
> {noformat}
> In the later for-loop we try to call *getPaletteIndex(rgb)* on the SimplePalette instance. If it contains null, we'll get a NullPointerException here.
> {noformat}
> for (int y = src.getHeight() - 1; y >= 0; y--) {
>             for (int x = 0; x < src.getWidth(); x++) {
>                 final int argb = src.getRGB(x, y);
>                 if (bitCount < 8) {
>                     final int rgb = 0xffffff & argb;
>                     final int index = palette.getPaletteIndex(rgb); // possible NullPointerException
>                    ...
>                 } else if (bitCount == 8) {
>                     final int rgb = 0xffffff & argb;
>                     final int index = palette.getPaletteIndex(rgb);  // possible NullPointerException
> {noformat}
> Why can SimplePalette be null? It comes from PaletteFactory::makeExactRgbPaletteSimple(final BufferedImage src, final int max). As it's javadoc says it will "fails by returning null if there are more than max colors necessary":
> {noformat}
> if (rgbs.add(rgb) && rgbs.size() > max) {
>                     return null;
>                 }
> {noformat}
> My first idea goes to throw a RunTimeException rather than returning null. But one has to check if there are cases where the return of null triggers some error handling i.e. increasing the number of colors or creating a different type of object.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)