[nycphp-talk] Code review
Leila Lappin
damovand at yahoo.com
Mon Jun 6 09:45:57 EDT 2005
Hi,
Thank you for your review. Interesting to know in how
many ways a code can fail. Im going to find a better
way to handle the points you mentioned.
Thanks again for your time and help.
--- Paul Reinheimer <preinheimer at gmail.com> wrote:
> 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
> _______________________________________________
> New York PHP Talk Mailing List
> AMP Technology
> Supporting Apache, MySQL and PHP
> http://lists.nyphp.org/mailman/listinfo/talk
> http://www.nyphp.org
>
__________________________________
Do you Yahoo!?
Yahoo! Mail - Helps protect you from nasty viruses.
http://promotions.yahoo.com/new_mail
More information about the talk
mailing list