CODEWITHSUNDEEP
c#.netsqlsql-server-2005
The Mask
The Mask

Reputation: 17457

Simple sql query not working

I tried to insert some data into my database (sql server/local file) but it doesn't work.

public bool SaveCookie(string cookie, string expires)
{
    SimpleDBM db = new SimpleDBM();
    db.Connect();
    try
    {
        string query = string.Format("INSERT INTO Cookies(cookie_value, cookie_expires) VALUES('{0}', '{1}');", cookie, expires);
        SqlCommand cmd = new SqlCommand();
        cmd.CommandText = query;
        //... 
        SqlDataReader data = db.Query(ref cmd);
        return data.Read();
    }
    catch
    {
        return false;
    }
    finally
    {
        db.Close();
    }
}

The SimpleDBM class:

public class SimpleDBM {

    public static string dbpath = @"...";
    public static string dbname = "db.mdf";
    public static string dfullPath = Path.Combine(dbpath, dbname);
    public static string connStr = string.Format(@"Data Source=.\SQLEXPRESS;AttachDbFilename={0};Integrated Security=True;Connect Timeout=30;User Instance=True", dfullPath);

    private SqlConnection con; 

    public void Connect()
    {
        con = new SqlConnection();
        con.ConnectionString = connStr;
        con.Open();
    }

    public SqlDataReader Query(ref SqlCommand cmd)
    {
        cmd.Connection = con;
        return cmd.ExecuteReader();
    }

    public void Close()
    {
        con.Close();
    }

}

Can someone point out my mistake? For other queries it seems to work fine.

Thanks in advance.

Upvotes: 0

Views: 1550

Answers (2)

Scott Chamberlain
Scott Chamberlain

Reputation: 127603

You are using a ExecuteReader when you should be using ExecuteNonQuery.

Not related to your error you really should not be using String.Format with SqlCommand. What you should do is

string query = "INSERT INTO Cookies(cookie_value, cookie_expires) VALUES(@cookie, @expires);", cookie, expires);
SqlCommand cmd = new SqlCommand();
cmd.Parameters.AddWithValue("@cookie", cookie);
cmd.Parameters.AddWithValue("@expires", expires);
cmd.CommandText = query;

With your method ask your self if someone passed a cookie of ' ''); Drop table Cookies --? This is called a "Sql Injection Attack" and is one of the top 5 reasons websites get hacked.

EDIT

Just to help give another example of why using String.Format to pass values you did not generate is bad. http://xkcd.com/327/

Upvotes: 3

Mike Dinescu
Mike Dinescu

Reputation: 55760

The problem seems to be that you're trying to execute a query that doesn't return a result set using the ExecuteReader method of the SqlCommand class which will attempt to execute your query and create and return a DataReader for an eventual result set.

You should use ExecuteNonQuery for INSERT and UPDATE sql statements.

SIDE NOTE

Not that it's the reason you're getting the error but you should also consider using SqlParamters instead of composing the values into the INSERT statement. Using prepared SQL statements generally gives a performance enhancement and also helps prevent SQL injection attacks.

For an example of using prepared statements, see the MSDN page or the Prepare method.

Upvotes: 5

Related Questions