Click here to Skip to main content
15,350,817 members
Please Sign up or sign in to vote.
4.00/5 (1 vote)
See more:
SqlConnection connection = new SqlConnection(ConnectionString.Conn);
connection.Open();
SqlCommand command = connection.CreateCommand();
command.CommandText = "UPDATE MS4 SET Views = Views+1 WHERE SheetName='" +textBox2.Text + "'"; //updates the notes coloumn in MS4
SqlDataReader dataread = command.ExecuteReader(); //executes command



this is the code I currently have, it doesn't work well for me.
what am I doing wrong please? I don't want it to be an auto increment.
Posted
Comments
dbrenth 24-May-11 13:30pm
   
Please define "it doesn't work well for me." That could mean anything, Including that the table keeps disappearing everytime you enter "' DROP TABLE MS4 --" in the textbox.
LighthouseCall 24-May-11 13:34pm
   
' DROP TABLE MS4 -- ? Didn't get that part.
By it doesn't work well for me, I mean that the table is fine, but the value does not increment as it should.
dbrenth 24-May-11 13:47pm
   
I am just warning you of potential sql injection attacks. You may want to look at using SqlParameter's instead of just blindly throwing in whatever a user types in at the text box.

Anyway, do you get a better result when you try command.ExecuteNonQuery();?

1 solution

As dbrenth has said: use ExecuteNonQuery rather than Executereader: it should work then.

The first comment he made refers to your concatenation of strings to form the SQL staement, and that it is a dangerous thing to do: Not only can it make code very difficult yo read, but it can also allow accidental or deliberate SQL injection accatks which can destroy your database.

In the example he gave, if your user types "x';DROP TABLE MS4--" and you submit your query as is, SQL will interpret it as two separate commands: An UPDATE command followed by a DROP TABLE command. It will then delete (with no undo facility) your entire MS4 table.

If you think this is unlikely, it is one of the first things people tried with the UK on-line census recently...and they were mostly upset when it didn't do any damage.

There are other malicious things you can do with it, but prevention is easy:

using (SqlConnection connection = new SqlConnection(ConnectionString.Conn))
   {
   connection.Open();
   using (SqlCommand command = new SqlCommand("UPDATE MS4 SET Views = Views + 1 WHERE SheetName=@SN",connection))
      {
      command.Parameters.AddWithValue("@SN", textBox2.Text);
      command.ExecuteNonQuery();
      }
   }
Please note that SQL connections and so forth are scarce resources, and you are responsible for disposing of those you create. The above is probably the easiest way.

[edit]Typo: should have been SqlCommand, typed SqlConnection :O - OriginalGriff[/edit]
   
v2
Comments
fjdiewornncalwe 24-May-11 14:53pm
   
100% agreement from me on this one.
LighthouseCall 25-May-11 3:15am
   
using (SqlCommand command = new SqlConnection...
I think you meant new SqlCommand right?
I'll try the code in a minute. Thanks.
OriginalGriff 25-May-11 3:26am
   
Fixed! Thanks - :O

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900