Input string was not in a correct format error not getting resolved for class function

Question

I am getting the error as

Input string was not in a correct format.

newRow["col_frm_bin_id"] = CF.ExecuteScaler("Select location_name from wms_storage_bin where mkey = " + e.Record["from_bin"] + "");


public string ExecuteScaler(string StrQuery)
{
    DB.EConnection();
    cmd = new SqlCommand(StrQuery, DB.conn);
    cmd.Connection = DB.conn;
    int val=Convert.ToInt32(cmd.ExecuteScalar());
    DB.conn.Close();
    string ret = val.ToString();
    return ret;
}

I tried with converting but still it didn't worked

The *very first* thing to fix is your huge SQL Injection Attack vulnerability. Use parameterized SQL, every time. Once you've done that, edit your question to make it a lot clearer where the exception occurs. I'd also strongly recommend that you start following .NET naming conventions, use `using` statements appropriately, and create new connections on-demand rather than having `DB.conn`... — Jon Skeet, Jan 20 '17 at 12:20
Why the empty string at the end of your argument for `ExecuteScaler`? — , Jan 20 '17 at 12:21
I'll try to push it in your mind..SQLParameters, **always use SQLParameters**, SQLParameters, SQLParameters, **you need SQLParameters**, `SQLParameters is the answer`. — Renatas M., Jan 20 '17 at 12:23
the problem is your field DataType is nvarchar so you should use `'` (single quote) but as Jon said. use sql parameter instead — esiprogrammer, Jan 20 '17 at 12:28
What does the SQL return? You're selecting `location_name`, and trying to convert it to an int? This seems fishy. — Lasse V. Karlsen, Jan 20 '17 at 12:40
This is only problem of not using Sql parameters. If you will use Sql parameter you will not get this kind of error anymore while executing Sql command — Fabio, Jan 20 '17 at 13:03

score 0 · Answer 1 · answered Jan 20 '17 at 12:21

0

Your return column name sounds like its a string variable, Change it with int type column, or remove Convert.ToInt32 from code side

public string ExecuteScaler(string StrQuery)
{
    DB.EConnection();
    cmd = new SqlCommand(StrQuery, DB.conn);
    cmd.Connection = DB.conn;
    string ret=cmd.ExecuteScalar().ToString();
    DB.conn.Close();
    return ret;
}

answered Jan 20 '17 at 12:21

kgzdev

2,770
2
18
35

Sorry cant change class function – Nad Jan 20 '17 at 12:23
Your method returns a string value, so remove convert.toint from code side, it will fix your problem – kgzdev Jan 20 '17 at 12:24

Meer · Answer 2 · 2017-01-20T12:37:15.033

0

i think you should do like this but this is not good practice and also not safe

your mkey value should be in between quotes

mkey = '" + e.Record["from_bin"] + "'

newRow["col_frm_bin_id"] = CF.ExecuteScaler("Select location_name from wms_storage_bin where mkey = '" + e.Record["from_bin"] + "'");

public string ExecuteScaler(string StrQuery)
{
 DB.EConnection();
 cmd = new SqlCommand(StrQuery, DB.conn);
 cmd.Connection = DB.conn;
 int val=Convert.ToInt32(cmd.ExecuteScalar());
 DB.conn.Close();
 string ret = val.ToString();
 return ret;
}

but sending parameters is best practice

edited Jan 20 '17 at 12:37

answered Jan 20 '17 at 12:27

Meer

2,765
2
19
28

1

[Please do not suggest this answer](https://en.wikipedia.org/wiki/SQL_injection). What if `from_bin` record contains something like this: `'; drop table wms_storage_bin where '1'='1` – Renatas M. Jan 20 '17 at 12:35
@Reniuz i have already mentioned in my answer that sending parameters will be good – Meer Jan 20 '17 at 12:36

score 0 · Answer 3 · edited May 23 '17 at 11:45

I'll try summarize various pieces of information from other answers and comments.

First, your existing code is open to Sql injections. This is a very bad thing. To avoid the risk of Sql injection you shoul use Parametrized queries. See for instance here.

That means your ExecuteScaler method should not take a string as its argument, but instead a SqlCommand (I have corrected the spelling of scalar):

public string ExecuteScalar(SqlCommand query) { ... }

Your current implementation of ExecuteScaler is also at risk of leaking SqlConnetions. If an exception is thrown in this method before the DB.conn.Close() line, the connection will not be closed. For instance, in the case you described in the question, the following line is the prime suspect:

int val = Convert.ToInt32(cmd.ExecuteScalar());

With your current call to the method, you seem to be fetching something that is a string from the database. Unless that string is convertible to Int32, this line will throw an exception, and the connection will not be closed. To fix this, you should at the minimum add a try { ... } finally { ... } block:

public string ExecuteScalar(SqlCommand query)
{
    try 
    {
        DB.EConnection();
        query.Connection = DB.conn;
        string ret = query.ExecuteScalar().ToString();
        return ret;
    }
    finally 
    {
        if(DB.conn.State == ConnectionState.Open)
            DB.conn.Close();
    }
}

I would also suggest that you create separate versions of ExecuteScalar for different expected return types. Perhaps:

public string GetStringScalar(SqlCommand query)
public int GetInt32Scalar(SqlCommand query)

etc.

The calling code then needs to be changed:

string locName = null;
using (SqlCommand locNameCommand = new SqlCommand(@"
    select location_name
    from wms_storage_bin
    where mkey = @mkey
    ")) 
{
    locNameCommand.Parameters.AddWithValue("mkey", e.Record["from_bin"]);
    locName = GetStringScalar(locNameCommand);
}
newRow["col_frm_bin_id"] = locName;

Input string was not in a correct format error not getting resolved for class function

3 Answers3