NYCPHP Meetup

NYPHP.org

[nycphp-talk] Code review

Paul Reinheimer preinheimer at gmail.com
Sun Jun 5 23:31:51 EDT 2005


hrm, I got so caught up arguing with the site you linked
that I forgot to look at your code :)

Four things that stick out during a quick look

1. In several places you echo out links to the user agent, but encode the
ampersand, I don't think you need to do this, since you are using it as 
a seperator, not a charecter (or such is my understanding).
eg.
doQueryIndex.inc
line 59
href="/anp/auth_man.php?event=doQueryLogin&loginAgent=doQueryLogin&target=doQueryIndex

2. You're using a global array to store data in physiology.php
$_REQUEST['issue'] = htmlspecialchars(urldecode($url_array[4]));

By storing processed data (in this case by urldecode) in a global
variable, you make impossible for another function to determine
later on wether or not the processing has occured. What if I passed
an issue paramater in either a GET or POST?

3. This looks really dangerous
   $app = $class . '.inc';
   include ($app);  
If url fopen wrappers are enabled I can include any file I want, from anywhere.
If you absolutly need to do something like this, base it on a switch or if 
structure, 
if $class == "something" include "something.inc" elseif
$class == "somethingelse" include "somethingelse.inc".

4. Your zip structure didn't include any directory structure, it's a 
good idea to keep your .inc files out of the document root. This way
they can't be viewed by end users, or executed unexpectedly.



paul



On 6/5/05, Leila Lappin <damovand at yahoo.com> wrote:
> Hi,
> 
> I don't know if making my code publicly available
> would help since my scripts might not be useful to
> anyone, so they might not get anyone's attention.  I
> try to make my stuff interesting in terms of coding
> style.  I definitely know what I need to do but having
> useful tips how to do certain things is something
> else.  I'm sending a few of my scripts (in a zipped
> file) anything you care to browse through and comment
> would be great.   I'm using PHP4.3.5 so there isn't
> much object oriented coding. That's too bad since I
> could really do it up with features like protected and
> private members.
> 
> Thank you in advance for your time, and in case you're
> interested in a comparison between Java and PHP I
> found this link that you may like
> http://www.tek271.com/articles/JavaOrPhp.html and find
> useful.
> 
> 
> --- Christopher Hendry <chendry at gmail.com> wrote:
> 
> > Is it code you can make publicly available, I'm sure
> > you'll get plenty
> > of feedback if you do  :)
> >
> > Otherwise, send me some snippets off list, I'd love
> > to see how a
> > C++/Java programmer is using PHP.
> >
> > Is it PHP5?
> >
> > C
> >
> >
> > --
> >
> > "When you do things right, people won't be sure
> > you've done anything at all."
> > _______________________________________________
> > New York PHP Talk Mailing List
> > AMP Technology
> > Supporting Apache, MySQL and PHP
> > http://lists.nyphp.org/mailman/listinfo/talk
> > http://www.nyphp.org
> >
> 
> 
> 
> __________________________________
> Yahoo! Mail Mobile
> Take Yahoo! Mail with you! Check email on your mobile phone.
> http://mobile.yahoo.com/learn/mail
> 
> _______________________________________________
> New York PHP Talk Mailing List
> AMP Technology
> Supporting Apache, MySQL and PHP
> http://lists.nyphp.org/mailman/listinfo/talk
> http://www.nyphp.org
> 
> 
> 


-- 
Paul Reinheimer
Zend Certified Engineer



More information about the talk mailing list