On Tue, 2009-12-08 at 17:32 +0100, Jochem Maas wrote:
> Hi Allen,
>
> gonna be a bit ruthless with you :).
>
> 1. your not filtering your input (your open to include being hacked)
> 2. your not validating or error checking (e.g. does the include file exist??)
> 3. keeping large numbers of content pages with numerical filenames is a maintenance
> nightmare and incidentally not very SEO friendly
> 4. your not doing much debugging (I guess) - try using var_dump(), echo, print_r(),
> etc all over your code to figure out what it's doing (e.g. var_dump($_GET, $_POST) and
> print("HELLO - I THINK \$_GET['page'] is set."))
>
> personally I never rely on relative paths - I always have the app determine a
> full path to the application root (either at install/update or at the beginning
> of a request)
>
> also I would suggest you use 1 include file for all your scripts (rather than
> per dir) ... copy/past code sucks (read up on the DRY principe).
>
> additionally look into FrontController patterns and the possibility to
> stuff all that content into a database which gives all sorts of opportunities
> for management/editing.
>
> <?php
>
> $page = isset($_GET['page']) && strlen($_GET['page'])
> ? basename($_GET['page'])
> : null
> ;
>
> if (!$page || !preg_match('#^[a-z0-9]+$#i', $page))
> $page = 'default';
>
> $file = dirname(__FILE__) . '/content/' . $page . '.inc';
>
> if (!file_exists($file) || !is_readable($file)) {
> error_log('Hack attempt? page = '.$page.', file = '.$file);
> header('Status: 404');
> exit;
> }
>
> // echo header
> include $file;
> // echo header
>
> ?>
>
> maybe I've bombarded you with unfamiliar concepts, functions and/or syntax.
> if so please take time to look it all up ... and then come back with questions :)
>
> have fun.
>
> Allen McCabe schreef:
> > I have been using includes for my content for a while now with no problems.
> > Suddenly it has stopped working, and it may or may not be from some changes
> > I made in my code structure.
> >
> > I use default.php for most or all of my pages within a given directory,
> > changing the content via page numbers in the query string.
> >
> >
> > So on default.php, I have the following code:
> >
> >
> > <?php
> > if(isset($_GET['page']))
> > {
> > $thispage = $_GET['page'];
> > $content = 'content/'.$_GET['page'].'.inc';
> > }
> > else
> > {
> > $thispage = "default";
> > $content = 'content/default.inc';
> > }
> > ?>
> > <html>, <body>, <div> etc.
> > <?php include($content); ?>
> >
> >
> > I have a content subdirectory where I store all the pages with files such as
> > "default.inc, 101.inc, 102.inc, etc.
> >
> > As I said, this has been working fine up until now, if I use the url
> > "user/default.php" or just "user/" I get this error:
> >
> >
> > *Warning*: include(content/.inc)
> > [function.include<http://lpacmarketing.hostzi.com/user/function.include>]:
> > failed to open stream: No such file or directory in *
> > /home/a9066165/public_html/user/default.php* on line *89*
> >
> > AND
> >
> > *Warning*: include()
> > [function.include<http://lpacmarketing.hostzi.com/user/function.include>]:
> > Failed opening 'content/.inc' for inclusion
> > (include_path='.:/usr/lib/php:/usr/local/lib/php') in *
> > /home/a9066165/public_html/user/default.php* on line *89*
> >
> > But if I use "user/default.php?page=default" I get the correct content.
> >
> > It's acting as if page is set, but set to NULL, and then trying to find an
> > include at path "content/.inc" what's going on??
> >
>
>
The SEO factor here is only minor. Very little weight is given to the
filename of a page, much more is given to the content and the way it is
marked up.
Thanks,
Ash
http://www.ashleysheridan.co.uk
