0

Can someone please tell me why I keep on getting "Invalid Input" on my code?? I've checked my database several times and I cant seem to find the problem. I'm using a normalized database right now.

I just noticed I pasted a wrong code

namespace MemorialSystem
{
    public partial class Reservation : Form
    {
        SqlConnection con;
        SqlCommand cmd;
        SqlDataAdapter adapter;
        SqlCommandBuilder cd;
        DataSet ds;

        public Reservation()
        {
            InitializeComponent();
        }

        private void button1_Click(object sender, EventArgs e)
        {
            Form1 o = new Form1();
            o.Show();
            this.Hide();
        }

        private void Reservation_Load(object sender, EventArgs e)
        {
            con = new SqlConnection("Data Source=(local);Initial Catalog=Memorial_park;Integrated Security=True");
            cmd = new SqlCommand("select * from Records", con);
            adapter = new SqlDataAdapter(cmd);
            cd = new SqlCommandBuilder(adapter);
            ds = new DataSet();
        }

        private void button2_Click(object sender, EventArgs e)
        {
            con.Open();
            try
            {

                if (textBox1.Text == "" || textBox2.Text == "" || textBox3.Text == "" || comboBox1.Text == "" || textBox8.Text == "" || dateTimePicker1.Value.ToString("yyyyMMdd HH:mm:ss") == "" || dateTimePicker2.Value.ToString("yyyyMMdd HH:mm:ss") == "" || textBox7.Text == "" || textBox5.Text == "" || dateTimePicker3.Value.ToString("yyyyMMdd HH:mm:ss") == "")
                {
                    MessageBox.Show("Please input a value!", "Error", MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
                }
                else
                {
                    if (MessageBox.Show("Are you sure you want to reserve this record?", "Reserve", MessageBoxButtons.YesNo, MessageBoxIcon.Question) == DialogResult.Yes)
                    {
                        cmd = new SqlCommand("insert into Records(NameofLotOwner, HomeAddress, TelNo, RelationDeceased, NameOfDeceased, Address, DateofBirth, DateofDeath, PlaceofDeath, CausefDeath, DateofInterment) values('" + textBox1.Text + "', '" + textBox2.Text + "', '" + textBox3.Text + "', '" + comboBox1.SelectedItem + "', '" + textBox8.Text + "', '" + dateTimePicker1.Value.ToString("yyyyMMdd HH:mm:ss") + "', '" + dateTimePicker2.Value.ToString("yyyyMMdd HH:mm:ss") + "', '" + textBox7.Text + "', '" + textBox5.Text + "', '" + dateTimePicker3.Value.ToString("yyyyMMdd HH:mm:ss") + "')", con);

                        cmd.ExecuteNonQuery();

                        MessageBox.Show("Your reservation has been made!", "Reserve", MessageBoxButtons.OK, MessageBoxIcon.Information);
                    }
                }
            }

            catch (Exception x)
            {
                MessageBox.Show("Invalid Input");
            }
            con.Close();
        }

        private void label16_Click(object sender, EventArgs e)
        {

        }
    }
}
Marc
  • 3,905
  • 4
  • 21
  • 37
Celine
  • 45
  • 1
  • 7
  • 5
    [Obligatory...](http://xkcd.com/327/) – Sergey Kalinichenko Sep 14 '13 at 14:45
  • 1
    when I see queries like this I feel like throwing up – meda Sep 14 '13 at 14:48
  • 5
    Even if you find the problem that you are trying to solve, don't do what you're doing: this is wrong on so many levels! Start with storing passwords in plain text: that's just wrong. At this point, your design is doomed without any hope. Read [this](http://stackoverflow.com/a/1054033/335858) before you go any further with the design. – Sergey Kalinichenko Sep 14 '13 at 14:49
  • While I totally agree with @dasblinkenlight, if we are going to help you it might be helpful if you told us which line was erroring out. I would guess that it's the ExecuteReader line... Also, if you just did a select statement without any where criteria, does the error happen? – wcm Sep 14 '13 at 14:55
  • I pasted the wrong code. The first code I pasted was working and I have no problems with that lol :) – Celine Sep 14 '13 at 14:57
  • Please see Steve answer, he shows the "right" way of using parametrized queries that is something that would make your code more readable and secure. Now, it is easy for you to see why your code is showing "invalid input" - try to put a breakpoint on the line where you call the MessageBox and when your code stops there, inspect the "x" - this is the exception that your code is calling. Another alternative, to debug, is to replace "Invalid Input" for `x.ToString()` - that whould show the real error happening. – Leo Sep 14 '13 at 15:05
  • ^ I tried x.ToString() and x.Message and it kept on saying Invalid Input. – Celine Sep 14 '13 at 15:26
  • Again, where is the error happening? What line specifically? – wcm Sep 14 '13 at 15:37
  • Try to comment (put a "//" in front of the line) your `try` and `catch` lines and run the code inside visual studio. When the exception happens, VS should stop and show the error. – Leo Sep 14 '13 at 15:41

1 Answers1

4

I suggest to use a parameterized query like this

   try
   {
        string cmdText = "select username, password from Login " + 
                         "where username=@uname and password=@pwd";
        using(SqlConnection con = new SqlConnection(.....))
        using(SqlCommand cmd = new SqlCommand(cmdText, con);
        {
            con.Open();
            cmd.Parameters.AddWithValue("@uname", textbox1.Text);
            cmd.Parameters.AddWithValue("@pwd", textbox2.Text);
            SqlDataReader reader = cmd.ExecuteReader();
            if (reader.Read())
            {
               ......
            }
        }
    {
    catch (Exception ex)
    {
         .....
    }

In this way, if you have a single quote in user name or password the syntax passed to the underlying engine will be correctly quoted by the framework code and you avoid Sql Injection (see link from dasblinkenlight in comments)

EDIT Now that you have updated your code, I think that my suggestion is more valid now than before.
Using a string concatenation to build a command is a very bad practice as you can see with all that quoting required by a moderate length statement like yours.
If you use the parameter collection of the SqlCommand you will avoid all this mess with quoting strings, decimals and datetime values.

As a side note, don't keep a global connection object open for the lifetime of your form. If you forget to close and dispose your program will start to leak resources and your application becomes unstable (See the using statement and Connection Pooling)

 cmd = new SqlCommand("insert into Records(NameofLotOwner, HomeAddress, TelNo, " + 
                      "RelationDeceased, NameOfDeceased, Address, DateofBirth, " + 
                      "DateofDeath, PlaceofDeath, CausefDeath, DateofInterment) " + 
                      "values(@p1, @p2, @p3,@p4, @p5 @p6, @p6, @p8, @p9,@p10, @p11)", con);
 cmd.Parameters.AddWithValue("@p1", textBox1.Text);
 .....
 cmd.Parameters.AddWithValue("@p6", dateTimePicker1.Value);
 .....
Steve
  • 213,761
  • 22
  • 232
  • 286