Conversion failed when converting the varchar to int mvc C#

Question

I'm getting a runtime error

Conversion failed when converting the varchar to int

with this code:

public ActionResult Index(Model1 m)
{
    string q = "select username, password from login where username=@username and password=@password ";

    con.Open();

    SqlCommand cmd = new SqlCommand(q, con);
    cmd.Parameters.AddWithValue("@username", m.username).ToString();
    cmd.Parameters.AddWithValue("@password", m.password).ToString();

    SqlDataReader dr = cmd.ExecuteReader();

    if (dr.Read())   // getting error at this point
    {
        return RedirectToAction("upload");
    }
    else
    {
        ViewData["Message"] = "invalid username or password";
    }

    con.Close();

    return View();
}

1. Don't use `AddWithValue`. 2. Don't use `ExecuteReader` for this. Instead, use `ExecuteScalar` (and select 1 instead of 2 columns). 3. It looks like you're storing plain text passwords - don't do that. Instead, store a salted hash. 4. Looks like you're using a field for SqlConnection - this is also a mistake 5. The couple of `.ToString()` you have there are pointless. — Zohar Peled, Mar 15 '20 at 06:27
You should be `using` your SqlCommand, not returning before you close the connection. All in there's a huge amount (nearly all) of this code that is questionable and it should probably all be thrown away and reworked — Caius Jard, Mar 15 '20 at 06:45
And, as a stupid little check, make sure that your username and password members are actually typed as strings. Here's a reference about why @zoharpeled says you shouldn't use `AddWithValue`: https://blogs.msmvps.com/jcoehoorn/blog/2014/05/12/can-we-stop-using-addwithvalue-already/ — Flydog57, Mar 15 '20 at 15:05

score -1 · Answer 1 · answered Mar 15 '20 at 06:38

Everything you need to do is in this answer including using Entity Framework instead of this ancient, long winded way of running SQLs that doesn't promote good OO design practices (separation of concerns between your business logic layer in your controller and your data access layer)

At the very least, take a look at switching to using Dapper for your data access - it'll make your code cleaner and your life a lot easier

Use the first linked answer to develop a class that salts and hashes your passwords and stores the hashes rather than the plain text passwords, and call on it ensuring your passwords are held securely. Storing passwords in plaintext is the reason why services like HaveIBeenPwned need to exist; people reuse passwords and when you store your passwords in plaintext and your system is hacked it will reveal a nice easy lookup of username:password that can be used elsewhere that has been broken into and all the hacker has is the username. Part of that is the users fault for reusing a password, but mostly your fault for not holding their details securely. Please prioritize this

Conversion failed when converting the varchar to int mvc C#

1 Answers1