Jump to content

Recommended Posts

Posted

hi guys,

 

having a bit of trouble with one of my scripts, the code is meant to store an image into a mysql table, where the field is of type LONGBLOB, here is the code for the form;

 

</pre><form enctype="multipart/form-data" name="uploadFile" action="<?php%20echo%20%24PHP_SELF;%20?>" method="post">


   
   
     
   Upload Picture:
     
     
       


     
   
<

 

and the code for the storing of the uploaded file:

 

//check a file has been uploaded
 if($_FILES['pic'] == "")
 {
   head("No Picture");
   echo "

   
     No Photo Selected 


     Try Again


   
   ";    
   footer();
   exit();
 }
$PSize = filesize($_FILES['pic']['name']); 
$mysqlPicture = addslashes(fread(fopen($_FILES['pic']['name'], "rb"), $PSize)); 

$sql = "UPDATE ifile SET 
`if_Photo` = '$mysqlPicture'
    WHERE `if_PU_Adno` = '$adno'";

mysql_query($sql)or die("Unable to save picture ". mysql_error() ."");

 

I am retrieving an error on these two lines:

 

$PSize = filesize($_FILES['pic']['name']); 
$mysqlPicture = addslashes(fread(fopen($_FILES['pic']['name'], "rb"), $PSize)); 

 

Errors:

 

Warning: filesize() [function.filesize]: stat failed for TEST.png in /public_html/upload.php on line 120

 

Warning: fopen(TEST.png) [function.fopen]: failed to open stream: No such file or directory in /public_html/upload.php on line 121

 

Warning: fread(): supplied argument is not a valid stream resource in/public_html/upload.php on line 121

 

ive checked that the following are set:

 

file_uploads = On

upload_tmp_dir = ""

upload_max_filesize = 24M

 

to my knowledge leaving upload_tmp_dir blank will set it to use the system default..

 

does anyone see any errors?

Posted
does anyone see any errors?

 

Yes, your plan. Do NOT do this, it's a monstrosity.

 

Save the file somewhere (after sanity checking until your hair turns grey, and following the usual rules about not storing a billion files in one directory - you know about that one, right?) with a unique name (try sha1 on the current microtime, that's pretty unique) and store the path.

 

 

edit: and why the hell are you calling addslashes on binary data? You should have checked what it is and discarded it way before then if it's dangerous...

Posted
Yes, your plan. Do NOT do this, it's a monstrosity.

 

1 Question, why?

 

Ive used blobs before to store other file types such as word documents, and excel files, without any issues or problems, this is the first to give me a problem. As for the addslashes its the general advice given for this method to escape any characters.

 

Storing the files in a directory with a unique name is easy enough to do, but i was hoping to use this method instead, unless someone can come up with some real reasons why i shouldnt?

 

If your worried about database overhead, any statements referring to the picture will NOT use "SELECT *" and all my tables are indexed properly for speed. Storing a file in a database like this makes backups easier and allows me to store the files in one place instead of storing location in one and file in another, also prevents any linkage problems if files are moved.

Posted

>Storing the files in a directory with a unique name is easy enough to do, but i was >hoping to use this method instead, unless someone can come up with some real >reasons why i shouldnt?

 

Just have a Google around - you'll find this is general considered bad practice except for very small images: To BLOB or Not To BLOB: Large Object Storage in a Database or a Filesystem - Microsoft Research.

 

>If your worried about database overhead, any statements referring to the picture will >NOT use "SELECT *" and all my tables are indexed properly for speed.

 

What about overheads in terms of database backup? Storing things as blob will make the DB bigger and so increase the time to backup and increase storage demands for backups. Even more so when often things like pictures don't actually change that often. So you could just rsynch the picture directory of elsewhere for backup.

 

Granted it does make backups easier going the blob route but at the detriment of everything else.

 

>also prevents any linkage problems if files are moved.

 

This shouldn't be a problem as you'll have a config file that declares a variable with the link path. If you change paths you edit that one variable and you're good to go again.

  • Thanks 1
Posted (edited)

Thankyou Danbuntu, quite an informative read that microsoft article, straight from the article,

 

The study indicates that if objects are larger than one megabyte on average, NTFS has a clear advantage over SQL Server. If the objects are under 256 kilobytes, the database has a clear advantage. Inside this range, it depends on how write intensive the workload is, and the storage age of a typical replica in the system.

 

Since i will be limiting the user to a file size of 1MB it seems that either method would suffice. However if i were to store larger amounts of data than this the FileSystem would be the clear choice.

 

Another interesting snippit out of that article:

 

Following the lead of existing benchmarks, we evaluated the read performance of the two systems on a clean data store. Figure 1 demonstrates the truth of the folklore: objects up to about 1MB are best stored as database BLOBS.

 

Performance of SQL reads for objects of 256KB was 2x better than NTFS; but the systems had parity at 1MB objects. With 10MB objects, NTFS outperformed SQL Server.

 

back to the original question, any idea whats worng with that code?

Edited by Flakes
Quote hadnt copied properly
Posted
As for the addslashes its the general advice given for this method to escape any characters.

 

It's a side issue, but it reflects less-than-optimum programming practice. You're inspecting binary data, which you can't add slashes to (funnily enough, it tends to corrupt it) and you should have checked for safety long before you reach this stage - like running it through your friendly neighbourhood virus scanner first

 

Storing the files in a directory with a unique name is easy enough to do, but i was hoping to use this method instead, unless someone can come up with some real reasons why i shouldnt?

 

It inflates the database unnecessarily, which makes recovery difficult

It inflates the indexes in a really ugly way

It inflates the binary log file in a really ugly way

It's impossible to monitor space use by individual files

It's impossible to share files across a cluster or replication partner without massive overhead

It's expensive to add large blobs to indexes, and if they're not indexed it's expensive to find them

A hit on an image costs you two lookups and two runs of the PHP parser, which is expensive - sending a file to the agent is the web server's job and it does it very, very fast

It's impossible to take backups that are not the entire dataset (because your database is a filesystem blob)

Your file system can do deduplication far more easily that you can

Your virus scanner can't inspect the stored files

You should be caching file requests on the server to protect against page refreshes, so you're going to end up storing them twice

File systems have strengths and weaknesses. You can't store your files on a different and more suitable file system to your database

 

also prevents any linkage problems if files are moved.

 

So don't move them.

 

Granted it does make backups easier going the blob route but at the detriment of everything else.

 

Not really, you can no longer do incremental or differential or partial backups.

Posted

$PSize = filesize($_FILES['pic']['name']); 
$mysqlPicture = addslashes(fread(fopen($_FILES['pic']['name'], "rb"), $PSize)); 

 

From the PHP manual:

 

$_FILES['userfile']['name']

The original name of the file on the client machine.

 

There is a very good reason that the original name is not kept: it's an attack vector to overwrite arbitrary files on the server. You should be using

 

$_FILES['userfile']['tmp_name']

The temporary filename of the file in which the uploaded file was stored on the server.

  • Thanks 1
Posted

Mabye this is my fault that i havnt given enough information, the "ifile" (the system im working on, is like an internal facebook, accessable by the students here only in school, the idea behind it is that the kids can add themselves to out of school activites and write a profile about them-selves its also a way of allowing the students to ask there own questions about the school which are then answered by form tutors. The profile picture(the part im having trouble with) is just to allow them to have a small image 100x100 that will give the kids some personalisation to there page.

 

It inflates the database unnecessarily, which makes recovery difficult

OK yes storing binary data will obviously increase the size of the database, no arguments there.

 

It inflates the indexes in a really ugly way

It inflates the binary log file in a really ugly way

 

done some searching and cant find anything on this, source?

 

It's impossible to monitor space use by individual files

 

Non issue file size will be limited to 1MB, and either way the files either take space up here or on the filesystem, plus its a paid for server with 50GB of space, think it can handle it, old students are deleted as they leave the school.

 

It's impossible to share files across a cluster or replication partner without massive overhead

 

As much as i would like one, we dont have a cluster or a replication partner, school wouldnt pay for one, they dont see why we need it. (ive optimised the system greatly, nearly all pages take less than 200ms to load, and the ones that dont are fpdf reports which take 1 second or so)

 

It's expensive to add large blobs to indexes, and if they're not indexed it's expensive to find them

A hit on an image costs you two lookups and two runs of the PHP parser, which is expensive - sending a file to the agent is the web server's job and it does it very, very fast

 

Again ive done alot of optimization with the Database/Server/Apache/MYSQL, and although you are correct that this will be more expensive on the PHP Parser and Indexes, the above document suggests that storing these small image files in a database is still the faster method to storing them in the filesystem. i may do some of my own testing on this issue when i have some spare time.

 

It's impossible to take backups that are not the entire dataset (because your database is a filesystem blob)

 

I always keep a local copy of the database on my computer, running with XAMPP to test all data and scripts before sending them to the online Server, Full backups are run every night, and are actually emailed to me and the Data Manager.

 

Your virus scanner can't inspect the stored files

 

Students can only access this in school, all computers have inbuilt virus scanner.

 

 

So don't move them.

 

This is short sited, if the server failed, or major system changes are being made(like what im currently doing), or we changed address/server, the pictures may need to be moved.

Posted
Mabye this is my fault that i havnt given enough information, the "ifile" (the system im working on, is like an internal facebook, accessable by the students here only in school,

 

Have you looked at elgg or mahara? you could also do some of this with Moodle or indeed Organic groups on Drupal or Community Manager under Joomla. Might be easier then reinventing the wheel

Posted
Mabye this is my fault that i havnt given enough information, the "ifile" (the system im working on, is like an internal facebook, accessable by the students here only in school, the idea behind it is that the kids can add themselves to out of school activites and write a profile about them-selves its also a way of allowing the students to ask there own questions about the school which are then answered by form tutors.

 

You're too late (screenshots).

Posted
ive never liked using cms systems that are already there, you learn so much more from "reinventing the wheel". Plus this way i can do what what i like with the system.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now



×
×
  • Create New...