4 Replies - 1686 Views - Last Post: 31 January 2012 - 10:49 AM

#1 webwired  Icon User is offline

  • D.I.C Regular
  • member icon

Reputation: 33
  • View blog
  • Posts: 339
  • Joined: 26-August 07

Programming Bad Practices?

Posted 30 January 2012 - 04:05 PM

So I thought I'd ask everyone here what you all think about 2 different things that I'm being accused of making bad programming practices on...

1.) Creating a directory on the fly when a new user signs up

2.) Saving files in directories vs. saving them directly in a database

Ok, so some more information... So this site I'm building, a potential User will Register, the registration process will use CAPTCHA or some other version of it, then before the site will allow this new potential user to log in, they must verify their email address from an email that was sent to them upon registration...

Once the now verified User has verified their email address, they can then proceed to the next step, which is to create a Profile, once they click on the "Create Profile" button, a new directory is created like this: "Content/user/UserName" ... Now when they registered for the new account, their potential new UserName was checked to make sure that it was not only unique, but checked against RegEx to ensure that it didn't contain any values other than numbers and letters and that it didn't exceed a certain length...

So that pretty much covers "1.)" therefore I ask, why is this bad practice, how is this harmful?

Now for more information regarding "2.)" ... The purpose of the User directory is to store their content, images and zip files for the most part... So when did it become bad practice to store files in a directory and store the file path in the database vs. storing the files directly in the database?

Anyway, I look forward to your replies...

Is This A Good Question/Topic? 0
  • +

Replies To: Programming Bad Practices?

#2 lordofduct  Icon User is offline

  • I'm a cheeseburger
  • member icon


Reputation: 2528
  • View blog
  • Posts: 4,630
  • Joined: 24-September 10

Re: Programming Bad Practices?

Posted 30 January 2012 - 05:39 PM

I see no issue with that personally. A user directory in tandem with a database entry for the user would make sense to me.

I've heard this ideology of database only before, I've always considered it like any extreme ideology... it's extreme. Someone does something stupid using some tool, and they're told what they did with the tool was stupid, and they take it to mean using the tool is stupid, and they perpetuate the idea that using the tool is stupid because they never figured out what they did with the tool was stupid.
Was This Post Helpful? 0
  • +
  • -

#3 wordswords  Icon User is offline

  • D.I.C Regular
  • member icon

Reputation: 76
  • View blog
  • Posts: 272
  • Joined: 17-December 11

Re: Programming Bad Practices?

Posted 30 January 2012 - 07:04 PM

Some points that might be relevant:

* A database copes with race conditions well. Creating two directories at the same time, within miliseconds of each other, could be more of a problem. This does happen in high traffic sites. Generally I am too lazy to bother worrying about such things, and just use a database.

* If you work in an enviroment where everyone is storing data in a database, it makes more sense to 'go with the flow' and use a database to store your data. It makes the environment easier to backup also, if everyone stores their data in the same way.

* Security is more of a risk if you have to validate data yourself. If I was using PHP, I would be far too lazy to worry about such things, and just use mysql_real_escape_string(). I am guessing there must be an equivalent for .NET. Don't do such things yourself, use tried and tested functions.

That said, I don't think you should store files in a database. I would always use a directory for those. This is what I would do:

1. Create a random hex string for the filename, 12 characters or so.
2. Check if this filename is not in use.
3. Copy the file to the 'files' folder with the name being the hex string plus extension, ie: /data/upload/203AFED3.PNG
4. Store this filename in the database, linked to the other data about this user.

I have seen this pattern repeated time and time again in PHP web development, and it seems to work.

This post has been edited by wordswords: 30 January 2012 - 07:10 PM

Was This Post Helpful? 3
  • +
  • -

#4 Atli  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 3710
  • View blog
  • Posts: 5,958
  • Joined: 08-June 10

Re: Programming Bad Practices?

Posted 30 January 2012 - 08:10 PM

View Postwebwired, on 30 January 2012 - 11:05 PM, said:

1.) Creating a directory on the fly when a new user signs up

I wouldn't consider this a "bad" practice, but I do sort of think it's unnecessary. I mean, in a dynamic website you typically make things like member restricted download pages with a single page that passes the content through from an offline location. - There is no need for a user specific directory when the page is driven by a database. Linking content to users is just a matter of setting a FK in whatever table stores the content data.

On the other hand, there isn't really anything wrong with using the user data to separate the files into directories. I mean:
/* If you're already doing this: */
string filePath = BASE_DIR + "/content/" + dbRow["file_id"];

/* This isn't really that different. */
string filePath = BASE_DIR + "/content/" + dbRow["user_name"] + "/" + dbRow["file_name"];


The only complication is having to actually create the directory, but I suppose that's not much of a complication :)

View Postwebwired, on 30 January 2012 - 11:05 PM, said:

So when did it become bad practice to store files in a directory and store the file path in the database vs. storing the files directly in the database?

Saving a big load of unformatted binary data into a relational database is usually a pretty bad idea. There are of course exceptions, like saving tiny icon images and such, but generally speaking you should let the file-system handle files. It's kind of what they are made for, and they do it extremely well. Relational databases, on the other hand, excel at small(ish) pieces of well formatted data, which binary files are definitely not.

There is some crossover though. Some types of NoSQL databases supposedly suffer less from taking in large files (haven't checked that myself though), and some RDBMSs try to make it easy and efficient to store files. MSSQL's FILESTREAM feature is one good example of this.

View Postwordswords, on 31 January 2012 - 02:04 AM, said:

* Security is more of a risk if you have to validate data yourself. If I was using PHP, I would be far too lazy to worry about such things, and just use mysql_real_escape_string(). I am guessing there must be an equivalent for .NET. Don't do such things yourself, use tried and tested functions.

The mysql_real_escape_string function has nothing to do with validation. It's ONLY meant to make SQL injection less likely. It's not a substitution for proper validation and sanitation of user input.

Validating a strict format like a username isn't exactly complicated. A simple regular expression like this would probably do: /([\w\d_'] ?){2,5}/i. White-list validation is always relatively simple.

Also, the mysql_real_escape_string function is not really what I'd call secure. Putting user input into a SQL query is always a risk, even if you escape it first. - Meanwhile, prepared statements allow you to pass the data separately from the query, bypassing any SQL injection risk altogether.

View Postwordswords, on 31 January 2012 - 02:04 AM, said:

That said, I don't think you should store files in a database. I would always use a directory for those. This is what I would do:

1. Create a random hex string for the filename, 12 characters or so.
2. Check if this filename is not in use.
3. Copy the file to the 'files' folder with the name being the hex string plus extension, ie: /data/upload/203AFED3.PNG
4. Store this filename in the database, linked to the other data about this user.

Why not just use an identity/auto_increment/serial field from the database as the filename? It makes just as much sense as a filename as a random hex string does (more even), and it ensures there won't ever be a collision. - Also, why bother appending the original file extension? You may as well leave it out. You'll have to use a download script to set the content-disposition for the download anyways.
Was This Post Helpful? 2
  • +
  • -

#5 wordswords  Icon User is offline

  • D.I.C Regular
  • member icon

Reputation: 76
  • View blog
  • Posts: 272
  • Joined: 17-December 11

Re: Programming Bad Practices?

Posted 31 January 2012 - 10:49 AM

Quote

The mysql_real_escape_string function has nothing to do with validation. It's ONLY meant to make SQL injection less likely. It's not a substitution for proper validation and sanitation of user input.

Validating a strict format like a username isn't exactly complicated. A simple regular expression like this would probably do: /([\w\d_'] ?){2,5}/i. White-list validation is always relatively simple.


Yes.. I meant it should be used for sanitising data, not validating it. Using regex to validate data is fine, but I got the impression that the OP was 'rolling his own' sanitisation method, and I don't condone that.

Quote

Also, the mysql_real_escape_string function is not really what I'd call secure. Putting user input into a SQL query is always a risk, even if you escape it first.


Why is this the case? Perhaps you could explain.

Quote

Why not just use an identity/auto_increment/serial field from the database as the filename? It makes just as much sense as a filename as a random hex string does (more even), and it ensures there won't ever be a collision. - Also, why bother appending the original file extension? You may as well leave it out. You'll have to use a download script to set the content-disposition for the download anyways.


Could do.. I think your solution is marginally better.

This post has been edited by wordswords: 31 January 2012 - 10:50 AM

Was This Post Helpful? 0
  • +
  • -

Page 1 of 1