Inserting a DateTime with SQL (while converting to string)

Question

So basically I am working with 2 layers, the first layer is called "BL" (stands for Business Logic) and the other layer is called "DAL".
I have a database which contains a users table (in one of its tables).

BL is accessing DAL through a reference (.dll) and DAL is accessing the Database (BL has no direct access to database).

I would like to receive user info on BL, send this to the DAL, which will insert it to the database.

My current methods:

BL:

 public bool InsertUser(string userFName, string userLName, string userPass, DateTime userBirth, string userEmail, string userCountry)
    {
        if (UsersDAL.Insert(userFName, userLName, userPass, userBirth.ToString(), userEmail, userCountry))
        {
            return true;
        }
        return false;
    }

DAL:

public static bool Insert(string userFName, string userLName, string userPass, string userBirth, string userEmail, string userCountry)
    {
        DateTime userBirthday = DateTime.Parse(userBirth);
        try
        {
            OleDbHelper.fill("INSERT INTO UsersTable(userFName, userLName, userPass, userBirth, userEmail, userCountry, userActive) VALUES('" + userFName + "', '" + userLName + "', '" + userPass + "', " + userBirthday + ", '" + userEmail + "', '" + userCountry + "', true)", "UsersTable");
        }
        catch
        {
            return false;
        }
        return true;
    }

The problem occurs with the Datetime thingy, so I can't find any solution for it.

Note: On the database, "userBirth" is a DateTime

Gah! Sql injection ahoy. Aldo: *don't treat date time a as strings* — Marc Gravell, Nov 02 '14 at 20:20
_What_ problem? Also, why do you convert the `DateTime` to `String` just to comvert it back to `DateTime` in the next method? Use sql-parameters as mentioned above. — Tim Schmelter, Nov 02 '14 at 20:20
when you go to try and find those with an upcoming birthday, you'll regret not leaving it as a `DateTime thingy` — Ňɏssa Pøngjǣrdenlarp, Nov 02 '14 at 20:22
You really need to parameterize that query. Not only will it solve the DateTime issue, but it will help protect you against SQL injection attacks, and could even improve performance by allowing the SQL execution plan to be reused. — Troy Gizzi, Nov 02 '14 at 20:28

score 2 · Answer 1 · answered Nov 02 '14 at 20:22

I believe you're trying to insert a String into a Date in your database. You should be able to convert the String to a date which fits the Date format in your database (maybe even removing the .toString() method on userBirth and changing the type of userBirth in the Insert method to a DateTime).

EDIT: try this instead of adding the datetime as a string in the middle of your query:

SqlCommand cmd = new SqlCommand("INSERT INTO <table> (<column>) VALUES (@value)", connection);
cmd.Parameters.AddWithValue("@value", dateTimeVariable);

score 2 · Answer 2 · answered Nov 02 '14 at 20:23

Basically, the fundamental problem here is treating datetimes as strings in the first place. That is unnecessary, except for a tiny tiny handful of cases. The more significant problem is the general practice of concatenating values into SQL, which is bad for all kinds of reasons. One of those reasons - actually one of the more minor ones - is the problems this has with formatting of numbers and dates.

Basically: use sql parameters. By adding the value as a parameter rather than as a concatenated string, it will all work correctly. You also won't get pwned as badly.

score 2 · Accepted Answer · answered Nov 02 '14 at 20:31

In your business layer, do not convert the DateTime value to a string using ToString():

public bool InsertUser(string userFName, string userLName, string userPass, DateTime userBirth, string userEmail, string userCountry)
{
    if (UsersDAL.Insert(userFName, userLName, userPass, userBirth, userEmail, userCountry))
    {
        return true;
    }
    return false;
}

Now in your data access layer, use DateTime for that parameter instead of string. Now the DateTime userBirthday = ... line is redundant:

public static bool Insert(string userFName, string userLName, string userPass, DateTime userBirth, string userEmail, string userCountry)
{
    try
    {
        OleDbHelper.fill("INSERT INTO UsersTable(userFName, userLName, userPass, userBirth, userEmail, userCountry, userActive) VALUES('" + userFName + "', '" + userLName + "', '" + userPass + "', " + userBirth + ", '" + userEmail + "', '" + userCountry + "', true)", "UsersTable");
    }
    catch
    {
        return false;
    }
    return true;
}

As a side note, you should modify your SQL command to use parameters. With your current string concatenation method, you are easily susceptible to SQL injection.

And what did you mean with your side note? Please provide an example. — OmerM25, Nov 02 '14 at 20:40
http://stackoverflow.com/questions/332365/how-does-the-sql-injection-from-the-bobby-tables-xkcd-comic-work — Ňɏssa Pøngjǣrdenlarp, Nov 02 '14 at 20:45
@Plutonix it gave me a good laugh but I have yet to understand how this relates to my project, if you could give a raw example I'd thank you. — OmerM25, Nov 02 '14 at 20:51
@BlackRainz If somebody enters SQL syntax into a name (`Robert'); DROP TABLE STUDENTS; --` in that example) and you forget to sanitize it, then you will execute that command when it's concatenated. The `');` ends your current command, and `--` turns the rest of the command into a comment. — grovesNL, Nov 02 '14 at 21:17

Inserting a DateTime with SQL (while converting to string)

3 Answers3