[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