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
– Marc Gravell, Commented Nov 2, 2014 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
– Tim Schmelter, Commented Nov 2, 2014 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
– Ňɏssa Pøngjǣrdenlarp, Commented Nov 2, 2014 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
– Troy Gizzi, Commented Nov 2, 2014 at 20:28

grovesNL · Accepted Answer · 2014-11-02 20:31:46Z

2

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.

answered Nov 2, 2014 at 20:31

grovesNL

6,1152 gold badges22 silver badges32 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

OmerM25 Over a year ago

And what did you mean with your side note? Please provide an example.

Ňɏssa Pøngjǣrdenlarp Over a year ago

stackoverflow.com/questions/332365/…

OmerM25 Over a year ago

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

grovesNL Over a year ago

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

syntax1993 · Accepted Answer · 2014-11-02 20:22:48Z

2

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

answered Nov 2, 2014 at 20:22

syntax1993

605 bronze badges

Comments

Marc Gravell · Accepted Answer · 2014-11-02 20:23:07Z

2

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.

answered Nov 2, 2014 at 20:23

Marc Gravell

1.1m273 gold badges2.6k silver badges3k bronze badges

Collectives™ on Stack Overflow

Inserting a DateTime with SQL (while converting to string)

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related