1

I am trying to check if a person has a past crime history or not by looking up his NIC number in criminal database. But the query is not fetching any data although there is no exception and relevant data is present in the database but still data reader is empty.

Please see the following code:

SqlCommand cmd5 = new SqlCommand("select * from criminal where NIC ="+nic, conn);
string nic = "null";

foreach (var person in allInvolved)
{
    conn.Open();
    nic = person.NIC;

    dr3 = cmd5.ExecuteReader();

    if (dr3.HasRows)
    { do something }
    else if (!dr3.HasRows)
    { do something else}
}

Variable NIC has the correct value in it, I checked it while debugging. I don't know where am I going wrong. If you need any other info regarding the code please ask.

Thanks in advance.

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
  • 1
    you need to call `read` on `dr3` after `ExecuteReader` line. – muratgu Jun 18 '16 at 23:23
  • You define nic as string - is it actually a string value or a number like you describe it as in problem. If it's alphanumeric, you'll have to wrap it in quotes in SQL. Sorry - that's all I can see that stands out for me – dbmitch Jun 18 '16 at 23:24
  • you mean like `if(dr3.Read())` ?? if yes then I have tried it and its not working. @muratgu – Syed Haris Ali Ghaznavi Jun 18 '16 at 23:25
  • You say that `nic` has the right value in it; but... what *is* that value? since this is a string, virtually all available values **will not work correctly**, since you have neither quoted nor parameterized. – Marc Gravell Jun 19 '16 at 00:17
  • I'm guessing you have more than one DML commands before this? – JC Borlagdan Jun 19 '16 at 00:33
  • yes you are right @JCBorlagdan – Syed Haris Ali Ghaznavi Jun 19 '16 at 00:42
  • did you try putting the data inside a dataset? – JC Borlagdan Jun 19 '16 at 04:03
  • @MarcGravell i have tried with this query `"select * from criminal where NIC like '%" + nic + "%'"` is this what you mean by quoting ? – Syed Haris Ali Ghaznavi Jun 19 '16 at 09:30
  • @SyedHarisAliGhaznavi well, sort of, but that is still a very bad way to do it. Basically, if you want to execute SQL with context-specific values, you have two choices: concatenation or parameterization. The above is a form of concatenation, but that is incredibly risky as it is a huge source of "SQL Injection" (SQLi) problems - causing both unexpected bugs and real security vulnerabilities. The preferred option is parameterization, which would be (for a similar example) `where NIC like '%' + @nic + '%'` - the difference is that now `@nic` is a parameter. We never inject the literal value... – Marc Gravell Jun 19 '16 at 10:11
  • ... the reason for this is that with concatenation, if the incoming value is "`O'conner's Network Card`", it will break horribly as the `'` in the text interrupts the meaning of the SQL. Now imagine that someone searches for "`whatever'; truncate table criminal; --`" - boom; you just lost your data. And this isn't just a theoretical risk: SQLi is the **number 1** vulnerability on most systems. There are tools like "havij" that even automate the process of discovery and exploitation. – Marc Gravell Jun 19 '16 at 10:13
  • @MarcGravell Ah ok. Got you. I was a bit familiar with SQL injections, you explained it quite well. thanks – Syed Haris Ali Ghaznavi Jun 19 '16 at 10:24

2 Answers2

2

When you do:

 SqlCommand cmd5 = new SqlCommand("select * from criminal where NIC ="+nic, conn);

the current value of nic is used at that moment; it doesn't matter what you change nic to afterwards:

nic = person.NIC;
dr3 = cmd5.ExecuteReader();

since that value is not used. Ideally, you should parameterize:

SqlCommand cmd5 = new SqlCommand(
    "select * from criminal where NIC = @nic", conn);
var param = cmd5.Paramters.Add("nic", SqlDbType.SomethingRelevant);
// ^^^ note: I don't know what the data type is, you'll need to pick that

foreach(...) {
    param.Value = ((object)person.NIC) ?? DBNull.Value;
    using(var dr3 = cmd5.ExecuteReader()) {
      // ...
    }
}
...

Additionally, note that a lot of these objects are IDisposable and should be in using blocks; alternatively, look at tools like "dapper" so you can do things like:

var criminals = conn.Query<Criminal>(
    "select * from criminal where NIC = @nic",
    new { nic = person.NIC}).AsList();
Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • Thanks for your reply. When i debug and take the cursor over `nic` in query, it shows the correct nic that it should be having in the query at that time. do you still think its the same problem that you described @Marc-Gravell – Syed Haris Ali Ghaznavi Jun 19 '16 at 00:29
  • @syed yes, basically – Marc Gravell Jun 19 '16 at 05:51
  • I implemented this solution and its giving the following exception on `executereader` line `The variable name '@nic' has already been declared. Variable names must be unique within a query batch or stored procedure.` – Syed Haris Ali Ghaznavi Jun 19 '16 at 09:56
  • @SyedHarisAliGhaznavi ah, right; then you'll need to declare it outside the loop; one moment, will edit (update: edited) – Marc Gravell Jun 19 '16 at 10:03
  • @SyedHarisAliGhaznavi it sounds like you're adding the parameter **inside the loop**; you need to add it *outside* the loop. And: that's the wrong data type; `SqlDbType.NChar` is fixed-length unicode (the `nchar` sql type); `varchar`, however, is variable-length ANSI. You want (oddly enough) `SqlDbType.VarChar` – Marc Gravell Jun 19 '16 at 11:05
  • Made the changes it still shows that there is no data in data reader after `cmd.executeReader()` i am doing this `string temp = dr3.GetString(dr3.GetOrdinal("NIC"));` and when i run this line gives the following exception `Invalid attempt to read when no data is present.` – Syed Haris Ali Ghaznavi Jun 19 '16 at 11:08
  • @SyedHarisAliGhaznavi if there's no rows, then there's no rows; either your query is wrong, or the data isn't what you think it is, or both. Since I can see neither your current data nor your current code, I can't tell you which. – Marc Gravell Jun 19 '16 at 11:10
  • yes i figured that out, have added parameter outside the loop and changed Nchar to VarChar – Syed Haris Ali Ghaznavi Jun 19 '16 at 11:12
  • But when I paste the same query in sql studio with same NIC number that is shown in nic variable it gives the desired result – Syed Haris Ali Ghaznavi Jun 19 '16 at 11:14
  • @SyedHarisAliGhaznavi again, I can't see the data and I can't see the code; pretty sure it isn't the RDBMS or the `SqlClient` library making the error, though – Marc Gravell Jun 19 '16 at 15:04
  • thanks for your help the problem was solved by the solution that u have mentioned above – Syed Haris Ali Ghaznavi Jun 20 '16 at 16:04
1

Try this

SqlConnection con = new SqlConnection(connectionString);  
SqlCommand cmd = new SqlCommand();  
cmd.Connection = con;  
con.Open();  

foreach (var person in allInvolved)
{
nic = person.NIC;
cmd.CommandText = "select * from criminal where NIC ="+nic;  
dr3 = cmd.ExecuteReader();

while (dr3.Read())  
{ 
      if (dr3.HasRows)
      { do something}
      else if (!dr3.HasRows)
      { do something else}
}

} con.Close();

neer
  • 4,031
  • 6
  • 20
  • 34
  • Just debug it please. Are you sure that you have data in db? @SyedHarisAliGhaznavi – neer Jun 18 '16 at 23:36
  • Yes, i debugged it the NIC variable had a value in it i copy and pasted that vale in SQL studio and executed the query and it gave the desired result, but with same parameters in ASP.NET it says that datareader is empty @NeerPriv – Syed Haris Ali Ghaznavi Jun 18 '16 at 23:39
  • Please check this. http://stackoverflow.com/questions/5794529/how-to-use-executereader-method-to-retrieve-the-value-of-just-one-cell @SyedHarisAliGhaznavi – neer Jun 18 '16 at 23:43