Re: [PEAR-CVS] cvs: pear /Image_Barcode ChangeLog Readme.txt barcode_img.php package.

On Wed, May 25, 2005 at 03:31:10PM -0000, Marcelo Subtil Marcal wrote:
> msmarcal		Wed May 25 11:31:10 2005 EDT
>
>   Added files:
>     /pear/Image_Barcode	ChangeLog Readme.txt barcode_img.php
>                        	package.xml test-image-barcode.php
>     /pear/Image_Barcode/Barcode	Code39.php ean13.php int25.php upca.php
>   Log:
>   Initial import into CVS


> +++ pear/Image_Barcode/Readme.txt
...
> = Reporting Bugs =
> ------------------
> Report bugs to Marcelo Subtil Marcal <msmarcal@php.net>

Instead, provide the URI to the bug interface on pearweb.


> http://cvs.php.net/co.php/pear/Imag...g.php?r=1.1&p=1
> Index: pear/Image_Barcode/barcode_img.php
> +++ pear/Image_Barcode/barcode_img.php
> <?php
>
> require_once("Image/Barcode.php");
>
> $num     = '15101967';
> $type    = 'int25';
> $imgtype = 'png';
>
> $num = isset($_REQUEST) && is_array($_REQUEST) && isset($_REQUEST['num']) 
? $_REQUEST['num'] : $num;
> $type = isset($_REQUEST) && is_array($_REQUEST) && isset($_REQUEST['type']
) ? $_REQUEST['type'] : $type;
> $imgtype = isset($_REQUEST) && is_array($_REQUEST) && isset($_REQUEST['img
type']) ? $_REQUEST['imgtype'] : $imgtype;
>
> Image_Barcode::draw($num, $type, $imgtype);

That example passes unchecked user input into a method.  Very poor
practice.

I then took a look at the draw() method and see some major problems
(http://cvs.php.net/co.php/pear/Imag...rcode.php?r=1.1)

@include_once("Image/Barcode/${type}.php");

Most importantly, you are not validating $type.  People can now pass
anything they want as type and include any file on the file system.

Second, the @ suppresses all errors inside the included file, not just the
include call itself.  Before doing the include, do a file_exists check and
return a PEAR error if the file doesn't exist.

The @since tag is used for versions of the package, not PHP.

I suspect the rest of the package has similar issues.


> +++ pear/Image_Barcode/package.xml
...
>       <file role="php" baseinstalldir="Image"  name="Barcode.php"/>

You'll need to add the replacements needed for the package version
numbers.  See the header comment block section of the coding standards.


> +++ pear/Image_Barcode/Barcode/Code39.php
...
> <?php
> /* vim: set expandtab tabstop=4 shiftwidth=4: */
> // +----------------------------------------------------------------------
+
> // | PHP version 4                                                        
|
> // +----------------------------------------------------------------------+[/color
]

Please use only the new header comment blocks.


>         if ( $wThick > 0 ) $this->_barthickwidth = $wThick;

Please adhere to the coding standards.


> 		/*
> 		 *
> 		 * make an image resource using the GD image library
> 		 *
> 		 * @param bool $noText Set to true if you'd like your barcode to be sans
 text
> 		 * @param int $bHeight height of the barcode image including text
> 		 * @return resource The Barcode Image (TM)
> 		 *
> 		 */
>     function plot( $noText = false, $bHeight = 0 ) {

You seem to be nesting with a combination of tabs and spaces.  Use only
spaces.  Plus, the opening of the docblock must be "/**", not "/*".

Thanks for your help in making PEAR great,

--Dan

--
T H E   A N A L Y S I S   A N D   S O L U T I O N S   C O M P A N Y
data intensive web and database programming
http://www.AnalysisAndSolutions.com/
4015 7th Ave #4, Brooklyn NY 11232  v: 718-854-0335 f: 718-854-0409