Question Validate 2 Textbox values and Retrieve 2 columns from SQL for a match

Kipp6912

New member
Joined
Dec 11, 2022
Messages
1
Programming Experience
Beginner
I have a SQL database I have been trying to write code that one textbox value = 192.168.1.1 and another textbox value = Main Area then i need it to check in SQL for those values in a record with matching values if it finds matching values then show message that values exist. If it doesn't find those 2 values then let the user insert the new values. this is what I have so far. It does find the IPaddress but never looks to see if the area is a mach in the database. If i change to an ip address that is not in the database I get 'Object reference not set to an instance of an object.' Thanks for any help i can get on this.
C#:
       private void Insert_Button_Click(object sender, EventArgs e)
        {
            SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["MainConn"].ConnectionString);
             conn.Open();
            string IPAddressQuery = "select ipaddress FROM Computer where IPAddress='" + IP_Address_TextBox.Text + "'";
            SqlCommand com = new SqlCommand(IPAddressQuery, conn);
            string ipaddress = com.ExecuteScalar().ToString();
            conn.Close();
            if (ipaddress == IP_Address_TextBox.Text)
                label4.Text =ipaddress.ToString();
                MessageBox.Show("Ip Address Found");
            {
                conn.Open();
                string checkAreaQuery = "select area from Computer where Area='" + Area_TextBox.Text + "'";
                SqlCommand areaComm = new SqlCommand(checkAreaQuery, conn);
                string area = areaComm.ExecuteScalar().ToString();
                if (area == Area_TextBox.Text)
                {
                    MessageBox.Show("IP Address is already in use for Area");
                }
                else
                {
                    Let the user insert a new record
                }
            }
        }
 

Attachments

  • IP address is the in database.jpg
    IP address is the in database.jpg
    276.6 KB · Views: 21
  • IP address not in database.jpg
    IP address not in database.jpg
    425.7 KB · Views: 60
Last edited by a moderator:
That is because com.ExecuteScalar() will return a null if nothing is found. Calling ToString() on null will throw the exception that is telling you exactly what you are doing wrong.

The correct thing to do is just get a count of matches and see if the count is greater than zero. Integers are not reference types and therefore will never be null.

As an aside what you are doing above by directly dropping in a user controlled string into a SQL query is setting yourself up for a SQL injection attack.

Obligatory XKCD cartoon:

exploits_of_a_mom.png
 
You shouldn't be calling ToString there anyway. If you expect something to return a string, you don't need to convert that to a string. If something is already a string then you should be casting, not converting:
C#:
var ipaddress = (string)com.ExecuteScalar();
The result will still be null but there will be no exception thrown. You'd just have to write subsequent code based on the fact that ipaddress might be null.
 
It does find the IPaddress but never looks to see if the area is a mach in the database.
That is because you you have two separate queries looking for two separate things. Perhaps write your query to look for both things at once. But that query is going to have to be written in SQL. This is a C# forum.

It's relatively easy though. In pseudo code it would be something like:
C#:
var query = "SELECT COUNT(*) WHERE ip = @ipParam AND area = @areaParam";
var command = new Command(query, connection);
command.AddParameterWithValue("@ipParam", txtIP.Text);
command.AddParameterWithValue("@areaParam", txtArea.Text);

var count = command.ExecuteScalar();
if (count > 0)
{
    we found at least on row where both the ip and area match
}
else
{
    we found no matches
}

line 1 is where the SQL is involved. Let that database do the hardwork of finding matches. The rest of the C# code is just to call the database the handle the results.
 
Last edited:
Code to validate your IP looks like an IP:

C#:
var bits = ipTextBox.Text.Split('.').Select(s => int.TryParse(s, out var i) ? i: -1).ToArray();
if(!bits.All(i => i >= 0 && i <= 255)){
  //it is not an IP
}
var cleanedIp = string.Join(".", bits);

If you fancy swapping to using EF Core for your database access your code looks like:

C#:
var existing = db.Computers.FirstOrDefault((c => c.IPAddress == cleanedIp && c.Area = areaTextBox.Text);
if(existing == default){
  //go ahead and add it
  db.Computers.Add(new Computer(){ ... set properties here ... });
  await db.SaveChangesAsync()
} else {
  //computer exists - show a message?
  MessageBox.Show($"Computer {existing.Name} already exsts with that IP in that area");
}

If you like writing SQLs, Dapper will make your life easier too:

C#:
var existing = connection.QueryFirstOrDefault<Computer>("SELECT * FROM Computers WHERE IPAddress = @ip AND Area = @a", new { ip = cleanedIp, a = areaTextBox.Text);
if(existing == default){
  //go ahead and add it
  connection.Execute("INSERT INTO Computers (name, ipaddress, area) VALUES (@n, @ip, @a)" new{ n = nameTextBox.Text, ip = cleanedIp, a = areaTextbox.Text });
} else {
  //computer exists - show a message?
  MessageBox.Show($"Computer {existing.Name} already exsts with that IP in that area");
}

Yes, all that painful, insecure code you wrote to get stuff from your DB can be reduced to a single line of easy, secure code.

Don't code like you are:

C#:
//NO, DEFINITELY NOT, EVER
"... where IPAddress='" + IP_Address_TextBox.Text + "'";

As soon as you put an apostrophe in the IP address box, that will explode with an sql error. As soon as it does, any script kid worth half the weight of his acne will know your system is massively vulnerable to SQL injection hacking, and it's game over for your security - you'll likely be hacked within minutes

"But it's not on the internet.." / "Noone would ever do that.." / "The users are trusted .." / "I'll just make sure there aren't.." are never reasonable excuses or solutions. Do it properly; I've shown above that is is far less work to do it properly than improperly, so it if's easier, faster and more secure, we should never do that thing where we take user input and concatenate it into an SQL string.
 

Latest posts

Back
Top Bottom