You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pdfbox.apache.org by "Peter_Lenahan@ibi.com (JIRA)" <ji...@apache.org> on 2009/01/24 20:23:59 UTC

[jira] Created: (PDFBOX-405) Not a bug, but definately incorrect code in PDPageContentStream

Not a bug, but definately incorrect code in PDPageContentStream
---------------------------------------------------------------

                 Key: PDFBOX-405
                 URL: https://issues.apache.org/jira/browse/PDFBOX-405
             Project: PDFBox
          Issue Type: Improvement
          Components: PDModel
         Environment: All
            Reporter: Peter_Lenahan@ibi.com


There is some deceiving code in the setStrokingColor(float [] components)  method. 
It is actually only used for ColorSpace.TYPE_GRAY

The result of the code is the impression that the other cases are handled here when they are not.

Here are the details.
package org.apache.pdfbox.pdmodel.edit;
public class PDPageContentStream

In this class this variable currentStrokingColorSpace never changes value.
    private PDColorSpace currentStrokingColorSpace = new PDDeviceGray();
// The javadoc is incorrect also it should say for the colorspace PDDeviceGray
    /**
     * Set the color components of current stroking colorspace.
     *
     * @param components The components to set for the current color.
     * @throws IOException If there is an error while writing to the stream.
     */
// 
// Some confusing code is below, this method is only called when 
// currentStrokingColorSpace is  ColorSpace.TYPE_GRAY
// This method should be made private, since it is only called within this class.
//
    public void setStrokingColor( float[] components ) throws IOException
    {
        for( int i=0; i< components.length; i++ )
        {
            appendRawCommands( formatDecimal.format( components[i] ) );
            appendRawCommands( SPACE );
        }
        //
        // The top half of this "if" statement is never executed 
        // the variable "currentStrokingColorSpace" is never set to 
        // anything other than a new class of type PDDeviceGray.
        // Therefore it can never be any of the 4 types of classes below.
        // 
        if( currentStrokingColorSpace instanceof PDSeparation ||
            currentStrokingColorSpace instanceof PDPattern ||
            currentStrokingColorSpace instanceof PDDeviceN ||
            currentStrokingColorSpace instanceof PDICCBased )
        {
            appendRawCommands( SET_STROKING_COLOR_COMPLEX );
        }
        else
        {
            appendRawCommands( SET_STROKING_COLOR_SIMPLE );
        }
    }
//When the top half of the IF statement is removed, then
//these variables are no longer used.
//
private static final String SET_STROKING_COLOR_COMPLEX="SCN\n";
      private PDColorSpace currentStrokingColorSpace = new PDDeviceGray();
// therefore I believe that you may never generate the value SCN in the PageContentStream.
----------------------------
//
// I searched all the code for this method; this is the only one reference to the method that I found.
// that is why I believe that the method should be made private.
// However; since it was public in the past, I can't guarantee that someone does not call the method.
//
   public void setStrokingColor( Color color ) throws IOException
    {
        ColorSpace colorSpace = color.getColorSpace();
        if( colorSpace.getType() == ColorSpace.TYPE_RGB )
        {
            setStrokingColor( color.getRed(), color.getGreen(),color.getBlue() );
        }
        else if( colorSpace.getType() == ColorSpace.TYPE_GRAY )
        {
            color.getColorComponents( colorComponents );
            setStrokingColor( colorComponents[0] ); // <-- this was the only reference to the method
        }



-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (PDFBOX-405) Not a bug, but definately incorrect code in PDPageContentStream

Posted by "Peter_Lenahan@ibi.com (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/PDFBOX-405?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Peter_Lenahan@ibi.com updated PDFBOX-405:
-----------------------------------------

    Description: 
There is some deceiving code in the setStrokingColor(float [] components)  method. 
It is actually only used for ColorSpace.TYPE_GRAY

This same description that I gave for this problem Exists for the other method. setNonStrokingColor

   public void setNonStrokingColor( float[] components ) throws IOException

The result of the code is the impression that the other cases are handled here when they are not.

Here are the details.
package org.apache.pdfbox.pdmodel.edit;
public class PDPageContentStream

In this class this variable currentStrokingColorSpace never changes value.
    private PDColorSpace currentStrokingColorSpace = new PDDeviceGray();
// The javadoc is incorrect also it should say for the colorspace PDDeviceGray
    /**
     * Set the color components of current stroking colorspace.
     *
     * @param components The components to set for the current color.
     * @throws IOException If there is an error while writing to the stream.
     */
// 
// Some confusing code is below, this method is only called when 
// currentStrokingColorSpace is  ColorSpace.TYPE_GRAY
// This method should be made private, since it is only called within this class.
//
    public void setStrokingColor( float[] components ) throws IOException
    {
        for( int i=0; i< components.length; i++ )
        {
            appendRawCommands( formatDecimal.format( components[i] ) );
            appendRawCommands( SPACE );
        }
        //
        // The top half of this "if" statement is never executed 
        // the variable "currentStrokingColorSpace" is never set to 
        // anything other than a new class of type PDDeviceGray.
        // Therefore it can never be any of the 4 types of classes below.
        // 
        if( currentStrokingColorSpace instanceof PDSeparation ||
            currentStrokingColorSpace instanceof PDPattern ||
            currentStrokingColorSpace instanceof PDDeviceN ||
            currentStrokingColorSpace instanceof PDICCBased )
        {
            appendRawCommands( SET_STROKING_COLOR_COMPLEX );
        }
        else
        {
            appendRawCommands( SET_STROKING_COLOR_SIMPLE );
        }
    }
//When the top half of the IF statement is removed, then
//these variables are no longer used.
//
private static final String SET_STROKING_COLOR_COMPLEX="SCN\n";
      private PDColorSpace currentStrokingColorSpace = new PDDeviceGray();
// therefore I believe that you may never generate the value SCN in the PageContentStream.
----------------------------
//
// I searched all the code for this method; this is the only one reference to the method that I found.
// that is why I believe that the method should be made private.
// However; since it was public in the past, I can't guarantee that someone does not call the method.
//
   public void setStrokingColor( Color color ) throws IOException
    {
        ColorSpace colorSpace = color.getColorSpace();
        if( colorSpace.getType() == ColorSpace.TYPE_RGB )
        {
            setStrokingColor( color.getRed(), color.getGreen(),color.getBlue() );
        }
        else if( colorSpace.getType() == ColorSpace.TYPE_GRAY )
        {
            color.getColorComponents( colorComponents );
            setStrokingColor( colorComponents[0] ); // <-- this was the only reference to the method
        }



  was:
There is some deceiving code in the setStrokingColor(float [] components)  method. 
It is actually only used for ColorSpace.TYPE_GRAY

The result of the code is the impression that the other cases are handled here when they are not.

Here are the details.
package org.apache.pdfbox.pdmodel.edit;
public class PDPageContentStream

In this class this variable currentStrokingColorSpace never changes value.
    private PDColorSpace currentStrokingColorSpace = new PDDeviceGray();
// The javadoc is incorrect also it should say for the colorspace PDDeviceGray
    /**
     * Set the color components of current stroking colorspace.
     *
     * @param components The components to set for the current color.
     * @throws IOException If there is an error while writing to the stream.
     */
// 
// Some confusing code is below, this method is only called when 
// currentStrokingColorSpace is  ColorSpace.TYPE_GRAY
// This method should be made private, since it is only called within this class.
//
    public void setStrokingColor( float[] components ) throws IOException
    {
        for( int i=0; i< components.length; i++ )
        {
            appendRawCommands( formatDecimal.format( components[i] ) );
            appendRawCommands( SPACE );
        }
        //
        // The top half of this "if" statement is never executed 
        // the variable "currentStrokingColorSpace" is never set to 
        // anything other than a new class of type PDDeviceGray.
        // Therefore it can never be any of the 4 types of classes below.
        // 
        if( currentStrokingColorSpace instanceof PDSeparation ||
            currentStrokingColorSpace instanceof PDPattern ||
            currentStrokingColorSpace instanceof PDDeviceN ||
            currentStrokingColorSpace instanceof PDICCBased )
        {
            appendRawCommands( SET_STROKING_COLOR_COMPLEX );
        }
        else
        {
            appendRawCommands( SET_STROKING_COLOR_SIMPLE );
        }
    }
//When the top half of the IF statement is removed, then
//these variables are no longer used.
//
private static final String SET_STROKING_COLOR_COMPLEX="SCN\n";
      private PDColorSpace currentStrokingColorSpace = new PDDeviceGray();
// therefore I believe that you may never generate the value SCN in the PageContentStream.
----------------------------
//
// I searched all the code for this method; this is the only one reference to the method that I found.
// that is why I believe that the method should be made private.
// However; since it was public in the past, I can't guarantee that someone does not call the method.
//
   public void setStrokingColor( Color color ) throws IOException
    {
        ColorSpace colorSpace = color.getColorSpace();
        if( colorSpace.getType() == ColorSpace.TYPE_RGB )
        {
            setStrokingColor( color.getRed(), color.getGreen(),color.getBlue() );
        }
        else if( colorSpace.getType() == ColorSpace.TYPE_GRAY )
        {
            color.getColorComponents( colorComponents );
            setStrokingColor( colorComponents[0] ); // <-- this was the only reference to the method
        }




> Not a bug, but definately incorrect code in PDPageContentStream
> ---------------------------------------------------------------
>
>                 Key: PDFBOX-405
>                 URL: https://issues.apache.org/jira/browse/PDFBOX-405
>             Project: PDFBox
>          Issue Type: Improvement
>          Components: PDModel
>         Environment: All
>            Reporter: Peter_Lenahan@ibi.com
>
> There is some deceiving code in the setStrokingColor(float [] components)  method. 
> It is actually only used for ColorSpace.TYPE_GRAY
> This same description that I gave for this problem Exists for the other method. setNonStrokingColor
>    public void setNonStrokingColor( float[] components ) throws IOException
> The result of the code is the impression that the other cases are handled here when they are not.
> Here are the details.
> package org.apache.pdfbox.pdmodel.edit;
> public class PDPageContentStream
> In this class this variable currentStrokingColorSpace never changes value.
>     private PDColorSpace currentStrokingColorSpace = new PDDeviceGray();
> // The javadoc is incorrect also it should say for the colorspace PDDeviceGray
>     /**
>      * Set the color components of current stroking colorspace.
>      *
>      * @param components The components to set for the current color.
>      * @throws IOException If there is an error while writing to the stream.
>      */
> // 
> // Some confusing code is below, this method is only called when 
> // currentStrokingColorSpace is  ColorSpace.TYPE_GRAY
> // This method should be made private, since it is only called within this class.
> //
>     public void setStrokingColor( float[] components ) throws IOException
>     {
>         for( int i=0; i< components.length; i++ )
>         {
>             appendRawCommands( formatDecimal.format( components[i] ) );
>             appendRawCommands( SPACE );
>         }
>         //
>         // The top half of this "if" statement is never executed 
>         // the variable "currentStrokingColorSpace" is never set to 
>         // anything other than a new class of type PDDeviceGray.
>         // Therefore it can never be any of the 4 types of classes below.
>         // 
>         if( currentStrokingColorSpace instanceof PDSeparation ||
>             currentStrokingColorSpace instanceof PDPattern ||
>             currentStrokingColorSpace instanceof PDDeviceN ||
>             currentStrokingColorSpace instanceof PDICCBased )
>         {
>             appendRawCommands( SET_STROKING_COLOR_COMPLEX );
>         }
>         else
>         {
>             appendRawCommands( SET_STROKING_COLOR_SIMPLE );
>         }
>     }
> //When the top half of the IF statement is removed, then
> //these variables are no longer used.
> //
> private static final String SET_STROKING_COLOR_COMPLEX="SCN\n";
>       private PDColorSpace currentStrokingColorSpace = new PDDeviceGray();
> // therefore I believe that you may never generate the value SCN in the PageContentStream.
> ----------------------------
> //
> // I searched all the code for this method; this is the only one reference to the method that I found.
> // that is why I believe that the method should be made private.
> // However; since it was public in the past, I can't guarantee that someone does not call the method.
> //
>    public void setStrokingColor( Color color ) throws IOException
>     {
>         ColorSpace colorSpace = color.getColorSpace();
>         if( colorSpace.getType() == ColorSpace.TYPE_RGB )
>         {
>             setStrokingColor( color.getRed(), color.getGreen(),color.getBlue() );
>         }
>         else if( colorSpace.getType() == ColorSpace.TYPE_GRAY )
>         {
>             color.getColorComponents( colorComponents );
>             setStrokingColor( colorComponents[0] ); // <-- this was the only reference to the method
>         }

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (PDFBOX-405) Not a bug, but definately incorrect code in PDPageContentStream

Posted by "Andreas Lehmkühler (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/PDFBOX-405?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Andreas Lehmkühler resolved PDFBOX-405.
---------------------------------------

       Resolution: Duplicate
    Fix Version/s: 0.8.0-incubator

This issue is already solved with PDFBOX-401.

> Not a bug, but definately incorrect code in PDPageContentStream
> ---------------------------------------------------------------
>
>                 Key: PDFBOX-405
>                 URL: https://issues.apache.org/jira/browse/PDFBOX-405
>             Project: PDFBox
>          Issue Type: Improvement
>          Components: PDModel
>         Environment: All
>            Reporter: Peter_Lenahan@ibi.com
>             Fix For: 0.8.0-incubator
>
>
> There is some deceiving code in the setStrokingColor(float [] components)  method. 
> It is actually only used for ColorSpace.TYPE_GRAY
> This same description that I gave for this problem Exists for the other method. setNonStrokingColor
>    public void setNonStrokingColor( float[] components ) throws IOException
> The result of the code is the impression that the other cases are handled here when they are not.
> Here are the details.
> package org.apache.pdfbox.pdmodel.edit;
> public class PDPageContentStream
> In this class this variable currentStrokingColorSpace never changes value.
>     private PDColorSpace currentStrokingColorSpace = new PDDeviceGray();
> // The javadoc is incorrect also it should say for the colorspace PDDeviceGray
>     /**
>      * Set the color components of current stroking colorspace.
>      *
>      * @param components The components to set for the current color.
>      * @throws IOException If there is an error while writing to the stream.
>      */
> // 
> // Some confusing code is below, this method is only called when 
> // currentStrokingColorSpace is  ColorSpace.TYPE_GRAY
> // This method should be made private, since it is only called within this class.
> //
>     public void setStrokingColor( float[] components ) throws IOException
>     {
>         for( int i=0; i< components.length; i++ )
>         {
>             appendRawCommands( formatDecimal.format( components[i] ) );
>             appendRawCommands( SPACE );
>         }
>         //
>         // The top half of this "if" statement is never executed 
>         // the variable "currentStrokingColorSpace" is never set to 
>         // anything other than a new class of type PDDeviceGray.
>         // Therefore it can never be any of the 4 types of classes below.
>         // 
>         if( currentStrokingColorSpace instanceof PDSeparation ||
>             currentStrokingColorSpace instanceof PDPattern ||
>             currentStrokingColorSpace instanceof PDDeviceN ||
>             currentStrokingColorSpace instanceof PDICCBased )
>         {
>             appendRawCommands( SET_STROKING_COLOR_COMPLEX );
>         }
>         else
>         {
>             appendRawCommands( SET_STROKING_COLOR_SIMPLE );
>         }
>     }
> //When the top half of the IF statement is removed, then
> //these variables are no longer used.
> //
> private static final String SET_STROKING_COLOR_COMPLEX="SCN\n";
>       private PDColorSpace currentStrokingColorSpace = new PDDeviceGray();
> // therefore I believe that you may never generate the value SCN in the PageContentStream.
> ----------------------------
> //
> // I searched all the code for this method; this is the only one reference to the method that I found.
> // that is why I believe that the method should be made private.
> // However; since it was public in the past, I can't guarantee that someone does not call the method.
> //
>    public void setStrokingColor( Color color ) throws IOException
>     {
>         ColorSpace colorSpace = color.getColorSpace();
>         if( colorSpace.getType() == ColorSpace.TYPE_RGB )
>         {
>             setStrokingColor( color.getRed(), color.getGreen(),color.getBlue() );
>         }
>         else if( colorSpace.getType() == ColorSpace.TYPE_GRAY )
>         {
>             color.getColorComponents( colorComponents );
>             setStrokingColor( colorComponents[0] ); // <-- this was the only reference to the method
>         }

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.