| Daniel Convissor 2005-05-26, 8:57 pm |
| 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['imgtype']) ? $_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 |
> // +----------------------------------------------------------------------+
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
|