NYCPHP Meetup

[Daniel Convissor]

Sir Nick:

You've got a lot of issues here.

On Wed, Apr 26, 2006 at 07:25:03PM -0400, Nick Aldwin wrote:
> 
> $filetempname=$_FILES['file']['tmp_name'];
> $filename=$_FILES['file']['name'];
> $filetype=$_FILES['file']['type'];
> 
> $query = "INSERT INTO `media` VALUES ('','$filename','$filetype')";

You're letting users inject SQL statements.

Also, you're delimiting your table name, via "`".  You should never use 
names that require delimiting.  To make sure of that, never delimit names.

Plus, name your columns in the insert query.  Makes things clearer here 
and keeps you from having to rewrite your application if you add another 
column to the table at some point in the future.

So, turn that into:

  $filename_sql = mysql_real_escape_string($filename);
  $filetype_sql = mysql_real_escape_string($filetype);
  $query = "INSERT INTO media (fname, ftype) VALUES
           ('$filename_sql','$filetype_sql')";


> $result = mysql_query($query);
> $query = "SELECT * FROM `media` WHERE `filename` = '$filename'";

Simlilar things, escape the SQL and ditch the delimiters:

  $query = "SELECT * FROM media WHERE filename = '$filename_sql'";


> $result = mysql_query($query);
> $newname = mysql_result($result,0,"id");

Does the "media" table have a unique key to prevent file names from being 
reused?  Otherwise your logic will fail, since you'll end up getting the 
id for the earlier file.

I wouldn't call that variable $newname, since it's not a name, just an ID.

More importantly than all the above, it looks like that id column in the 
"media" table is an auto-increment.  Right?  If so, you don't even need 
that select query.  You can get the id by calling mysql_insert_id().


> move_uploaded_file( $filetempname, "PLACE/".$newname.".dat");
> mysql_close($link);
> header( "location: PAGE.PHP" );

That should be capitalized and use a fully qualified URI, and you should 
use single quotes since it's not being evaluated (doesn't have variables 
in it):

  header('Location: http://host/dir/PAGE.PHP');


By the way, your coding standard is all over the place in terms of 
spacing, quoting and variable placement.  It helps in the long term to be 
consistent.


> *Warning*: move_uploaded_file(PLACE/8.dat): failed to open stream: No such
> file or directory in *BLAHBLAH/upload.php* on line *17*

So, does PLACE exist?  Does the web server have permissions to that 
location?  Are you running in safe mode or have a restriction on open base 
dir?

--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