Query value to textbox

Question

1.00/5 (3 votes)

See more:

I have result

System.Data.SqlClient.SqlDataReader

Some help?

What I have tried:

private void ukupno_bez_pdv_roba()
       {
           SqlConnection con2 = new SqlConnection(cs);

           string sqlquery = ("select * from mp_racun_roba where tip_robe = 'Roba (Generalno)' and id=" + id_fakture);


           SqlCommand command = new SqlCommand(sqlquery, con2);
           con2.Open();
           SqlDataReader sdr = command.ExecuteReader();

           roba_bez_pdvTextBox.Text = sdr.ToString();
           con2.Close();
       }

Posted 31-May-18 9:34am

Goran Bibic

Updated 1-Jun-18 7:55am

v2

Add a Solution

Comments

Richard Deeming 31-May-18 15:39pm

How many times?!

Everything you wanted to know about SQL injection (but were afraid to ask) | Troy Hunt[^]
How can I explain SQL injection without technical jargon? | Information Security Stack Exchange[^]
Query Parameterization Cheat Sheet | OWASP[^]

F-ES Sitecore 1-Jun-18 4:24am

That's not how you use SqlDataReader, look at the docs for an example

https://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqldatareader(v=vs.110).aspx

If you know your query can only return one row then select an explicit field ("select fieldname from ..." rather than "select * from ...") and use ExecuteScalar rather than ExecuteReader. Check the docs to see how to use that too.

2 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Richard Deeming · Answer 1 · 2018-05-31T09:44:00

Solution 1

C#

private void ukupno_bez_pdv_roba()
{
    using (SqlConnection con2 = new SqlConnection(cs))
    using (SqlCommand command = new SqlCommand("select YOUR_FIELD_NAME from mp_racun_roba where tip_robe = 'Roba (Generalno)' and id = @id", con2))
    {
        command.Parameters.AddWithValue("@id", id_fakture);
        
        con2.Open();
        using (SqlDataReader sdr = command.ExecuteReader())
        {
            if (sdr.Read())
            {
                roba_bez_pdvTextBox.Text = sdr.GetString(0);
            }
        }
    }
}

ADO.NET Overview | Microsoft Docs[^]
ADO.NET code examples | Microsoft Docs[^]

And once again, since you seem to keep forgetting this:
Everything you wanted to know about SQL injection (but were afraid to ask) | Troy Hunt[^]
How can I explain SQL injection without technical jargon? | Information Security Stack Exchange[^]
Query Parameterization Cheat Sheet | OWASP[^]

Interactive SQL Injection demo[^]

Hacking is child's play - SQL injection with Havij by 3 year old[^]

Posted 31-May-18 9:44am

Richard Deeming

Comments

Eric Lynch 31-May-18 16:10pm

Sigh...forgetting SQL injection does seem to be a theme in these posts. After answering a previous post, I seconded your concerns...sad to see its ongoing :(

F-ES Sitecore 1-Jun-18 4:26am

How do you know the code is liable to SQL injection? We can't see enough of it to know for sure.

Richard Deeming 1-Jun-18 7:17am

Have you seen his(?) previous questions?

Even if you haven't, it's a safe bet that someone using string concatenation instead of parameters won't only be doing that with "safe" values.

F-ES Sitecore 1-Jun-18 8:19am

Probably, but we still can't say for sure.

Richard Deeming 1-Jun-18 8:32am

As I said, look at the OP's previous questions. There's a definite pattern!

F-ES Sitecore 1-Jun-18 8:35am

I did look, that doesn't mean this code is liable to SQL injection though. The message being given out here is that concatenation is what makes your code liable to SQL injection whereas that's not the case and could give people the wrong idea about when their code is safe or not.

Richard Deeming 1-Jun-18 8:38am

Expecting novice developers to understand the subtle difference between "safe" and "unsafe" concatenation in SQL queries is rather optimistic.

And why should anyone bother? There are very few cases where it's not trivial to use parameters, and parameters are always "safe".

F-ES Sitecore 1-Jun-18 8:45am

You (or more importantly the person I actually replied to :) ) still don't know if the code was liable to SQL injection, that's all I'm saying.

Richard Deeming 1-Jun-18 8:47am

Always better to assume it is and tell them to use parameters, than to assume it isn't and tell them it's OK to concatenate the data into the command. :)

F-ES Sitecore 1-Jun-18 8:56am

Better still to tell them it might be liable and it's always better to use params than than telling them that it is liable and to use params. One is factually correct and good advice, the other factually incorrect even if still good advice.

Eric Lynch 1-Jun-18 9:54am

The world's an uncertain place. That said, developing a habit of not using parameters offers no real benefit and a pretty huge downside.

Based on the OPs posts, I'd be willing to wager that this downside bites him eventually. Normally, since he's been advised, this would be fine with me.

However, in this case, other folks (his users) potentially also suffer consequences. Because of this, I strongly support Richard's attempt to avert that outcome. He first answered the question and only afterwards added a caution.

Likewise, I can't prove that driving a car blindfolded will cause a crash, but I would certainly advise against it :)

F-ES Sitecore 1-Jun-18 10:01am

That's beside the point, you said his code was open to SQL injection when you don't know if it actually was.

Your comment about him at least answering the question is an important one in this case, as 9/10 I see people being berated for not using params without addressing the actual question, or people posting the use of params as an actual solution when it doesn't even fix the original problem. It's ok to add it as an "and by the way...." but that's not always done.

Eric Lynch 1-Jun-18 10:42am

Ummm...in fairness, if you re-read my post I never actually did say his code was open to SQL injection. I did say that forgetting about it is a theme in the posts. Based solely on the OPs posting history, I stick by that assertion.

Regarding your other point, I am similarly annoyed by respondents who sermonize without answering the question. I've been a recipient of a few of these replies.

However, this was certainly not the case here, in Richard's solution. He answered the question AND offered great advice. So, I stick by the support I offered there as well.

Its never fun offering advice (or apparently support :)). Richard did so, in my opinion, in exactly the correct fashion (answer first and then advise).

I'd also point out that the OP was free to respond. Although, he has ZERO obligation to do so.

If it were me, after having a number of my questions answered and seeing similar concerns raised from well-intentioned folk, I might say something like: "Hey, thanks for answering the question. I hear you about SQL injection, but don't worry it in this instance, because...<insert reason here>".

The absence of a such a response worries me a little bit. Again, he has ZERO obligation to respond.

But, I still get to worry. What can I say? I'm a worrier :)

Personally, I hope Richard continues to fight the good fight (so long as he continues to answer first :)). Too many companies end up the target of this so-easy-to-prevent attack.

Anyhow, that's about as good a job as I can do clarifying my original response, so I'll probably not be responding further. Nobody ever accused me of having great communication skills :)

F-ES Sitecore 1-Jun-18 10:56am

Cool story bro, but as I said, no-one knows if his code was open to SQL injection or not.

Goran Bibic · Answer 2 · 2018-06-01T07:57:00

Solved

SqlConnection con2 = new SqlConnection(cs);

           string sqlquery = ("SELECT SUM(isnull(cast(REPLACE(TRY_CONVERT(int,TRY_CONVERT(float,iznos_bpdv),1), '#,0.00','')AS decimal(10,2)),0.00)) as UKUPNObpdv," +
                                     " SUM(isnull(cast(REPLACE(TRY_CONVERT(float, TRY_CONVERT(float, pdv), 1), '#,0.00', '')AS decimal(10, 2)), 0.00)) as UKUPNOpdv," +
                                     " SUM(isnull(cast(REPLACE(TRY_CONVERT(float, TRY_CONVERT(float, iznos_sa_pdv), 1), '#,0.00', '')AS decimal(10, 2)), 0.00)) as UKUPNOsapdv" +
                                     " from mp_racun_roba" +
                                     " where tip_robe = 'Roba (Generalno)' and id_fakture =" + id_fakture
                             );


           SqlCommand command = new SqlCommand(sqlquery, con2);
               con2.Open();
               SqlDataReader sdr = command.ExecuteReader();

               if (sdr.Read())
               {

               roba_bez_pdvTextBox.Text = sdr["UKUPNObpdv"].ToString();
               roba_pdvTextBox.Text = sdr["UKUPNOpdv"].ToString();
               roba_sa_pdvTextBox.Text = sdr["UKUPNOsapdv"].ToString();

           }
               con2.Close();