What am I doing wrong with this C# .NET WinForms application?

Question

I am trying to edit an Access DB_. For some reason I cannot insert anything. I believe my code is correct. The connection string is correct (though for security purposes I put a fake one for this post). At the end, I do not get the MessageBox like I am supposed to at the end of the function. Nothing was added to the Access DB either.

Any reason why this might be?

namespace TestBuild
{
    public partial class Form1 : Form
    {

        OleDbConnection con = new OleDbConnection(@"Provider=Microsoft.ACE.OLEDB.12.0;Data Source=C:\Users...\Documents\TestDB.accdb");

        public Form1()
        {
            InitializeComponent();
        }
        private void Button1_Click(object sender, EventArgs e)
        {

            con.Open();
            OleDbCommand cmd = con.CreateCommand();
            cmd.CommandType = CommandType.Text;
            cmd.CommandText = "insert into table1 values('"+textBox1.Text+"','"+textBox2.Text+"')";
            cmd.ExecuteNonQuery();
            con.Close();
            MessageBox.Show("record inserted successfully");  
        }
    }
}

Add a try / catch and see if there is an exception being thrown. Or run your code in debug mode and step through it... — devlin carnate, Jun 27 '18 at 22:29
Possible duplicate of [SQL Injection prevention with Microsoft Access and VB.NET](https://stackoverflow.com/questions/16759516/sql-injection-prevention-with-microsoft-access-and-vb-net) — mjwills, Jun 27 '18 at 22:33
Are you sure that your event handler has been called? If you don't have any exceptions on that code then perhaps your code has not been called. Put a breakpoint on the first line and confirm that your event handler runs — Steve, Jun 27 '18 at 22:41
If you don't want to use the debugger, add a messagebox call as the first line in that click handler: do you see that message when you click the button? — Hans Keﬆing, Jun 28 '18 at 06:05
@arekenny3 - any luck getting things working? Did you refactor to add a try/catch block (important)? Were you able to step through the code and identify the exact point of failure? Using the debugger, did you confirm that Button1_Click() is actually being called? — paulsm4, Jun 28 '18 at 23:43
Yes. Unusual though. When I went to the [Design], I double clicked on the button and it auto made a button1_Click_1 function for me. My question, If i were to delete the function and make the function from scratch named the same thing, would it be the same? The reason why I ask this is because I am wondering if there is anything else that I don't know about going on in the background of my code? Is it pointing to a variable without me knowing it in the background. I think if I were to delete & create from scratch with no change, I think it would be pointing to the same variable. Correct? — arekenny3, Jun 29 '18 at 14:15
I think you're asking how your "Button1_Click(()" function is tied to the actual button-press event. If you look in your "Form.Designer.cs", you should see a line like this: `this.Button1.Click += new System.EventHandler(this.Button1_Click);` So yes, you can "delete and create from scratch" Or create a completely different function, if you want. Just be sure to [delegate](https://www.akadia.com/services/dotnet_delegates_and_events.html) with `+=`. PS: please "upvote" and/or "accept" my response if it was helpful. — paulsm4, Jun 29 '18 at 16:36

paulsm4 · Accepted Answer · 2018-06-28T04:28:28.530

Suggestion - please consider refactoring your code as follows, and step through it, a line at a time, in the MSVS debugger:

    string connString = @"Provider=Microsoft.ACE.OLEDB.12.0;Data Source=C:\Users...\Documents\TestDB.accdb";

    private void Button1_Click(object sender, EventArgs e)
    {
        string sql = "insert into table1 values('" + textBox1.Text + "','" + textBox2.Text + "')";
        OleDbCommand cmd= new OleDbCommand(sql);
        using (OleDbConnection con = new OleDbConnection(connString)) {
            cmd.Connection = conn;
            try
            {
                con.Open();
                cmd.ExecuteNonQuery();
                MessageBox.Show("record inserted successfully");
            }
            catch (Exception ex)
            {
                MessageBox.Show("ERROR" + ex.Message);
            }
        }
   }

PS:

If you wanted to use prepared statements, you'd change your code to something like this:

string sql = "insert into table1 values(@param1, @param2)";
...
cmd.Parameters.AddWithValue("@param1", textBox1.Text);
cmd.Parameters.AddWithValue("@param1", textBox2.Text);
con.Open();
cmd.Prepare();     
cmd.ExecuteNonQuery();

You can read more about techniques and guidelines for mitigating SQL injection here:

https://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet

Here is another good article:

Best Practices for Using ADO.NET (MSDN)

This code is just as vulnerable to Sql injection as the original. — stuartd, Jun 28 '18 at 00:22
Agree with StuartD, edit the answer to put the parameterised version as the only option. Also the `cmd` is `IDisposable` so should be in a `using` block. And you may want to take a look at [Can we stop using AddWithValue](https://blogs.msmvps.com/jcoehoorn/blog/2014/05/12/can-we-stop-using-addwithvalue-already/). — Richardissimo, Jun 28 '18 at 05:13

What am I doing wrong with this C# .NET WinForms application?

1 Answers1