[jira] [Comment Edited] (PDFBOX-2505) ArrayIndexOutOfBoundsException in PDColor constructor

["John Hewson (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/PDFBOX-2505?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14227857#comment-14227857 ] 

John Hewson edited comment on PDFBOX-2505 at 11/27/14 5:46 PM:
---------------------------------------------------------------

The method checkArgumentSize doesn't really belong in OperatorProcessor as it isn't used elsewhere, it would be better to move it to SetColor to avoid unnecessary abstraction. Also what happens to colours which have _too many_ components, couldn't these be truncated rather than ignored?


One last, thing, regarding the following change:
{code}
     /**
      * Returns the name of this operator, e.g. "BI".
+     * @return operator name.
      */
     public abstract String getName();
{code}


You don't need to worry about adding @return for simple getters, or any case where the text is essentially a duplicate of the description. The [Google Java Style Guide|https://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s7.2-summary-fragment] briefly mentions this, and it's becoming a standard practice now in newer Java codebases.


was (Author: jahewson):
The method {{checkArgumentSize}} doesn't really belong in OperatorProcessor as it isn't used elsewhere, it would be better to move it to SetColor to avoid unnecessary abstraction. Also what happens to colours which have _too many_ components, couldn't these be truncated rather than ignored?


On last, thing, regarding the following change:
{code}
     /**
      * Returns the name of this operator, e.g. "BI".
+     * @return operator name.
      */
     public abstract String getName();
{code}


You don't need to worry about adding @return for simple getters, or any case where the text is essentially a duplicate of the description. The [Google Java Style Guide|https://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s7.2-summary-fragment] briefly mentions this, and it's becoming a standard practice now in newer Java codebases.

> ArrayIndexOutOfBoundsException in PDColor constructor
> -----------------------------------------------------
>
>                 Key: PDFBOX-2505
>                 URL: https://issues.apache.org/jira/browse/PDFBOX-2505
>             Project: PDFBox
>          Issue Type: Bug
>          Components: Rendering
>    Affects Versions: 2.0.0
>            Reporter: Tilman Hausherr
>             Fix For: 2.0.0
>
>         Attachments: PDFBOX-2505-032618-p96.pdf
>
>
> {code}
> Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: -1
>         at java.util.ArrayList.elementData(Unknown Source)
>         at java.util.ArrayList.get(Unknown Source)
>         at org.apache.pdfbox.cos.COSArray.get(COSArray.java:210)
>         at org.apache.pdfbox.pdmodel.graphics.color.PDColor.<init>(PDColor.java:54)
>         at org.apache.pdfbox.contentstream.operator.color.SetColor.process(SetColor.java:41)
>         at org.apache.pdfbox.contentstream.operator.color.SetNonStrokingDeviceCMYKColor.process(SetNonStrokingDeviceCMYKColor.java:38)
> {code}
> The attached file has a "k" without arguments.
> This is only in 2.0, not in 1.8. In 1.8 SetNonStrokingCMYKColor initializes the array with size 4 (ok, it will crash if there are 5 arguments), in 2.0 SetNonStrokingDeviceCMYKColor / SetColor take what is there.
> Two possible solutions in SetColor:
> 1) initialize "components" with the initial colors of the colorspace
> 2) initialize "components" with empty array
> Both solutions get rid of the exception. Solution 2 is used in another constructor.
> Which one is better? (I'd prefer solution 1 because it has the correct array size and would also change the other constructor)



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)