SqlDataReader causes the webpage not to load

Question

I am using a database to relate a Department's Name with it's unique mailing code i.e. Accounting has mailing code 1337. ShortName and ID are the fields of the database that are being queried.

The Schema is

ID int
ShortName nvarchar(255)

This is done on the backend of a webpage using C#. I have used a SqlDataReader object elsewhere in the same code but no error was caused by this. I think I have a syntax error somewhere or I am trying to stick a string into an int or something silly but I can't see it.

The SqlConnection is set up correctly as it is used for a query upon page load and works when the indicated line is commented out.

SqlDataReader SQl_Reader;
string cmdString = "SELECT ShortName FROM departments WHERE (ID = " +
  department_Text.SelectedValue.ToString() + ")"; //a selected value in department_text would be the mailing code such as 1337, or 1304.

SqlCommand SQl_Command2 = new SqlCommand(cmdString, SQl_Connection);

SQl_Reader = SQl_Command.ExecuteReader();

string deptName = SQl_Reader["ShortName"].ToString();//This should assign the deptName = the value in SQl_Reader but it causes the page not to load. Not sure what is wrong with it

SQl_Reader.Close();
SQl_Connection.Close();

I have tried this multiple different ways even using a proprietary Connection, Command, and Reader just for this query. I have used a SqlDataReader for a similar purpose with the same two fields in another function of the code and it works perfectly. Any insights would be very appreciated.

You have a pretty serious sql injection vulnerability in this code. You need to be using parameterized queries instead. Most likely the value of `department_Text` is not what you think it is, and the query is not returning any results, therefore throwing an exception when you call `ToString()` on a null reference. — asawyer, Jun 24 '13 at 20:50
shouldn't you call SQL_Reader.Read() before reading anything? — Lukas Häfliger, Jun 24 '13 at 20:53

score 2 · Accepted Answer · edited Jun 24 '13 at 21:00

If you're going to use a reader, you need to call Read() before getting the values.

If you get want a single value, then you're better off using ExecuteScalar() instead.

Also use a parameterized query to protect against SQL injection (and other benefits too)..

    SqlDataReader SQl_Reader;
    string cmdString = "SELECT ShortName FROM departments WHERE ID = @ID;"
    SqlCommand SQl_Command2 = new SqlCommand(cmdString, SQl_Connection);
    SQl_Command2.Parameters.Add("@ID", department_Text.SelectedValue);
    SQl_Reader = SQl_Command.ExecuteReader();
    SQl_Reader.Read();
    string deptName = SQl_Reader["ShortName"].ToString();

    SQl_Reader.Close();
    SQl_Connection.Close();

or better..

    SqlDataReader SQl_Reader;
    string cmdString = "SELECT ShortName FROM departments WHERE ID = @ID;"
    SqlCommand SQl_Command2 = new SqlCommand(cmdString, SQl_Connection);
    SQl_Command2.Parameters.Add("@ID", department_Text.SelectedValue);
    SQl_Reader = SQl_Command.ExecuteReader();
    SQl_Reader.Read();
    string deptName = SQl_Command.ExecuteScalar() as String;

    SQl_Reader.Close();
    SQl_Connection.Close();

score 2 · Answer 2 · answered Jun 24 '13 at 20:57

You need to .Read() from the SQl_Reader before it will have any data. normally you would do

While (SQl_Reader.Read() )
{
   // Get values here
}

If you're certain there will only be one row returned, then you should change your query to

Select top 1 ShortName FROM

But you still need the Read(). Also I agree with the comment about being open to an injection attacks. you need to pass department_Text.SelectedValue in as a query parameter

score 1 · Answer 3 · edited May 23 '17 at 12:05

I've assumed that ID is primary key, so no other possible values for specyfic ID, then ExecuteScalar will do the job.

string cmdString = "SELECT ShortName FROM departments WHERE (ID = @depId)";

using (SqlCommand SQl_Command2 = new SqlCommand(cmdString, SQl_Connection))
{
    //a selected value in department_text would be the mailing code such as 1337, or 1304.
    SQl_Command2.Parameters.Add("@depId", System.Data.SqlDbType.Int).Value = department_Text.SelectedValue;

    string deptName = (string)SQl_Command2.ExecuteScalar();
}

Don't forget to use SqlCommand.Parameters Property instead of string concatenation.

I'ts good to use using to automatically dispose objects, more about it: Is SqlCommand.Dispose() required if associated SqlConnection will be disposed?.

SqlDataReader causes the webpage not to load

3 Answers3