Missed the point...



  • I'll let this one speak for itself.  I think I'm going to cry a little now...

    <FONT color=#0000ff size=2><FONT color=#0000ff size=2>private</FONT></FONT><FONT size=2> </FONT><FONT color=#008080 size=2><FONT color=#008080 size=2>DataSet</FONT></FONT><FONT size=2> getData(</FONT><FONT color=#0000ff size=2><FONT color=#0000ff size=2>string</FONT></FONT><FONT size=2> rptId)</FONT></FONT></FONT><FONT size=2>

    {

    </FONT><FONT color=#008080 size=2><FONT color=#008080 size=2>String</FONT></FONT><FONT size=2> sqlTxt = </FONT><FONT color=#800000 size=2><FONT color=#800000 size=2>"exec usp_StoredProcName "</FONT></FONT><FONT size=2> + rptId ;</FONT></FONT><FONT size=2>

    </FONT><FONT color=#008080 size=2><FONT color=#008080 size=2>DataSet</FONT></FONT><FONT size=2> ds = </FONT><FONT color=#0000ff size=2><FONT color=#0000ff size=2>new</FONT></FONT><FONT size=2> </FONT><FONT color=#008080 size=2><FONT color=#008080 size=2>DataSet</FONT></FONT><FONT size=2>();

    ds = </FONT><FONT color=#008080 size=2><FONT color=#008080 size=2>SqlHelper</FONT></FONT><FONT size=2>.ExecuteDataset(</FONT><FONT color=#008080 size=2><FONT color=#008080 size=2>GlobalConst</FONT></FONT><FONT size=2>.ConnectionString, System.Data.</FONT><FONT color=#008080 size=2><FONT color=#008080 size=2>CommandType</FONT></FONT><FONT size=2>.Text, sqlTxt);</FONT></FONT><FONT size=2>

    </FONT><FONT color=#0000ff size=2><FONT color=#0000ff size=2>return</FONT></FONT><FONT size=2> ds;

    }

    </FONT>

     



  • What's the problem? They're using sprocs.



  • @toth said:

    What's the problem? They're using sprocs.

    Oh wait, I see the problem, they're using global variables too.



  • I also forgot to add that rptId comes in from a query string parameter...



  • @toth said:

    What's the problem? They're using sprocs.
    Actually, that would've been a much better title...



  • @C-Octothorpe said:

    I also forgot to add that rptId comes in from a query string parameter...

    So your first post also missed the (main) point!

     



  • Yichk!  I hate it when people re-implement vtables as SQL sprocs.



  • I have over 20 years of programming experience, mostly in C and assembly. So I'm no gd dummy. But you database programmers should explain the WTF in your posts because they are very obtuse. Your posts are like telling me a joke in Swahili: I can't laugh because I don't understand it.



  • He's using stored procs to mitigate a SQL injection attack, but he's not actually adding the parameters as parameters, just combining sql together.  Since the string comes from the web end, he could put in any SQL he wants, allowing him to put (something like) "1; delete from users;"



  • @SilentRunner said:

    I have over 20 years of programming experience, mostly in C and assembly. So I'm no gd dummy. But you database programmers should explain the WTF in your posts because they are very obtuse.

    The Real WTF (the one not mentioned in the OP) is that rptId is passed-in via a URL parameter. Meaning, there's an obvious SQL insertion security bug.

    The other WTF is that this code calls a stored procedure which then returns the text of another SQL command to run. Basically, it adds an extraneous layer which does nothing. And kind of misses the point of having stored procedures in the first place. Edit: Oops, not certain about this one without seeing what SQLHelper does.

    @SilentRunner said:

    Your posts are like telling me a joke in Swahili: I can't laugh because I don't understand it.

    Jamaa mpenda wake za watu siku moja alikuwa akila uroda na mke wa jirani mtaa wa pili usiku wakati mumewe hayupo. Ghafla, mume karudi na kuanza kupiga hodi mlango wa mbele, mke kusikia mumewe karudi, kahamaki na kukimbilia kumtoa jamaa kupitia mlango wa nyuma. Jamaa alikurupuka mbio, akaruka ukuta akiwa uchi wa mnyama akakimbia hadi nyumbani kwake. Alipofika kwa mkewe akamwambia kapigwa na majambazi njiani wakamvua nguo zote na kumwibia kila kitu. Mkewe akamwambia "pole mpenzi lakini hawa majambazi si watu wema kabisa, yaani wamekuvua nguo zote na kukuvalisha condom!"



  • @blakeyrat said:

    @SilentRunner said:
    I have over 20 years of programming experience, mostly in C and assembly. So I'm no gd dummy. But you database programmers should explain the WTF in your posts because they are very obtuse.
    The Real WTF (the one not mentioned in the OP) is that rptId is passed-in via a URL parameter. Meaning, there's an obvious SQL insertion security bug.
    int reportId = int.Parse(Request["reportId");

    voila, no injection avenue.  What does this have to do with being in the URL?  Or are you just saying it's more out in the open if you don't have to look at the post params?

    @blakeyrat said:

    @SilentRunner said:
    Your posts are like telling me a joke in Swahili: I can't laugh because I don't understand it.
    Jamaa mpenda wake za watu siku moja alikuwa akila uroda na mke wa jirani mtaa wa pili usiku wakati mumewe hayupo. Ghafla, mume karudi na kuanza kupiga hodi mlango wa mbele, mke kusikia mumewe karudi, kahamaki na kukimbilia kumtoa jamaa kupitia mlango wa nyuma. Jamaa alikurupuka mbio, akaruka ukuta akiwa uchi wa mnyama akakimbia hadi nyumbani kwake. Alipofika kwa mkewe akamwambia kapigwa na majambazi njiani wakamvua nguo zote na kumwibia kila kitu. Mkewe akamwambia "pole mpenzi lakini hawa majambazi si watu wema kabisa, yaani wamekuvua nguo zote na kukuvalisha condom!"

    Google translated it and I still didn't get it.  Do you actually know what the joke is, or did you c/p?

  • ♿ (Parody)

    @blakeyrat said:

    The Real WTF (the one not mentioned in the OP) is that rptId is passed-in via a URL parameter. Meaning, there's an obvious SQL insertion security bug.

    No way. TRRWTF is calling the constructor to instantiate a throw away instance:

    DataSet ds = new DataSet();
    

    ds = SqlHelper.ExecuteDataset(GlobalConst.ConnectionString, System.Data.CommandType.Text, sqlTxt);


  • ♿ (Parody)

    @Sutherlands said:

    What does this have to do with being in the URL?

    Seriously? User input vs, say, some hard coded literals from your code?



  • @Sutherlands said:

    int reportId = int.Parse(Request["reportId");

    voila, no injection avenue.

    If reportIds are ints, we don't know that at all, and the fact that it's passed-in as a string doesn't imply that. I can only see the code that's pasted here. Besides, they're using .net. it does parameterization "for free". It's almost harder to execute raw SQL.

    @Sutherlands said:

    What does this have to do with being in the URL?

    Any user can change any URL to whatever they like. You never, ever trust anything from the client, the client is a liar.

    @Sutherlands said:

    Or are you just saying it's more out in the open if you don't have to look at the post params?

    Equally bad, either way you never trust the client. The client is a liar.

    @Sutherlands said:

    Do you actually know what the joke is, or did you c/p?

    Both.



  • @boomzilla said:

    @Sutherlands said:
    What does this have to do with being in the URL?
    Seriously? User input vs, say, some hard coded literals from your code?
    No, being in the URL versus being in a post.

    @blakeyrat said:

    Equally bad, either way you never trust the client. The client is a liar.

    Right, so we know that you have to sanitize your input.  Were you saying what boom was saying... that it should be in config instead of passed in?  Assuming it has to be passed in (they can get different reports), what does being in the URL have to do with anything?  There are ways to mitigate it, and it has nothing to do with the URL, unless you're using security through obscurity.

    edit: I'm saying that just because it's in the URL (or passed in any other way) doesn't mean there is a SQL injection attack available.

    @blakeyrat said:

     @Sutherlands said:
    Do you actually know what the joke is, or did you c/p?
    Both.
    So what is it? I have to know why they were trying to clothe the condoms.



  • @Sutherlands said:

    Right, so we know that you have to sanitize your input. Were you saying what boom was saying... that it should be in config instead of passed in? Assuming it has to be passed in (they can get different reports), what does being in the URL have to do with anything? There are ways to mitigate it, and it has nothing to do with the URL, unless you're using security through obscurity.

    As you imply, there's nothing wrong with taking directions from the URL (Get, Post, or whatever). I didn't say you don't use information from the client, I said you don't trust it. So as you imply, you must first sanitize it somehow.

    Fortunately, .net does this super-easily. These jokers just aren't using that capability.

    @Sutherlands said:

    @blakeyrat said:
     @Sutherlands said:
    Do you actually know what the joke is, or did you c/p?
    Both.
    So what is it? I have to know why they were trying to clothe the condoms.

    Both! The two options you gave weren't mutually-exclusive, so the answer is "both." And they left behind clothes and condoms.


  • ♿ (Parody)

    @Sutherlands said:

    @boomzilla said:
    @Sutherlands said:
    What does this have to do with being in the URL?

    Seriously? User input vs, say, some hard coded literals from your code?

    No, being in the URL versus being in a post.

    I'm not sure why that distinction matters. Either one can be changed from whatever you expect "good" data to be. Also, for the record, the hardcoded thing would be its own WTF, just not a security WTF.



  • @Sutherlands said:

    So what is the joke? I have to know why they were trying to clothe the condoms.

    FTFM

    So the man comes home to some men "robbing" the place, runs them off, and his naked wife says they left behind clothes and condoms? K.



  • I think, from viewing some of the alternate translations, that the man is off boffing someone else's wife when her husband comes home. She sends him out the back door, naked, and he does a runner. When he gets home he tells his wife that he was attacked by a gang of thieves, who stole his clothes as well. She says "They're strange thieves - they took all your clothes, but put a condom on!"



  • @Scarlet Manuka said:

    I think, from viewing some of the alternate translations, that the man is off boffing someone else's wife when her husband comes home. She sends him out the back door, naked, and he does a runner. When he gets home he tells his wife that he was attacked by a gang of thieves, who stole his clothes as well. She says "They're strange thieves - they took all your clothes, but put a condom on!"

    It's the way you tell 'em, I guess.

    Do we know for sure that no escaping is happening here? I mean, we can't see inside the sproc, so we're basically guessing? Or is it required to do the escaping outside the sproc? And if so, why?



  • @bertram said:

    Do we know for sure that no escaping is happening here? I mean, we can't see inside the sproc, so we're basically guessing? Or is it required to do the escaping outside the sproc? And if so, why?

    Doesn't matter.  You do SQL-injection protection at the point the client makes a database call.  Escaping inside the proc cannot fix the problem that you can inject SQL into the EXEC statement itself.

    This is real simple -- use parameters, always.  Anything else is potentially vulnerable to SQL injection.  Someone mentioned that the protection could be put in the page by casting the query string parameter as an int, but that misses the point.  Why would you ever create a DB layer method that exposes a potential SQL injection vulnerability when it is so easy to protect against it?



  • @Jaime said:

    This is real simple -- use parameters, always.
    This.

    I also hate it when stored procedures are used to store a set of SQL to execute, kinda like the DB version of a virtual method call.  Maintaining that is annoying as hell, because I don't get compile-time checks on the actual SQL I have to maintain.

    I think I'm going to submit a WTF on that very vein for the front page.



  • @SilentRunner said:

    I have over 20 years of programming experience, mostly in C and assembly. So I'm no gd dummy. But you database programmers should explain the WTF in your posts because they are very obtuse. Your posts are like telling me a joke in Swahili: I can't laugh because I don't understand it.

    You're right, my bad...  I should've explained what's going on here.

    1) URL parameter (unsanitized) is passed directly into an SQL exec statement

      1.1) The DB user in this case runs as system admin, therefore they can use extended stored procs which opens a whole new level of pwnage possibilities (think command line execution)...

    2) The rptId should be an integer (int.Parse), but is passed in as a string

    3) No use of parameters even though calling a stored procedure (kind of like using a can-opener as a hammer to open a can of beans)

    4) Instantiation of a DataSet object even though the call to SqlHelper returns a new instance of DataSet

    5) Connection string is held in GlobalConst class (Hey, we need to change the DB instance, OK? That's fine, we need to redeploy the code... What?!)

      5.1) What makes this a more of a WTF is the fact that it's a string constant, which is visible if you were to take a look at the binaries (username\password for SA user).

    6) String contatenation (nit-picking) of exec statement

    7) Bad naming convention on method name (though this can be ignored because the method is private)

    So there is plenty here to cause a good old fashioned head-desk...

    I'll be sure to include a description for future posts.  Sorry guys.



  • @SilentRunner said:

    I have over 20 years of programming experience, mostly in C and assembly. So I'm no gd dummy. But you database programmers should explain the WTF in your posts because they are very obtuse. Your posts are like telling me a joke in Swahili: I can't laugh because I don't understand it.
     

    TRWTF is that you have 53 posts here, yet you still apparently can't spot an obvious SQL injection vulnerability.  Some 31% of all WTFs posted here involve SQL injection, so I'm not entirely sure why you'd still need an explanation.



  • @Jaime said:

    @bertram said:

    Do we know for sure that no escaping is happening here? I mean, we can't see inside the sproc, so we're basically guessing? Or is it required to do the escaping outside the sproc? And if so, why?

    Doesn't matter.  You do SQL-injection protection at the point the client makes a database call.  Escaping inside the proc cannot fix the problem that you can inject SQL into the EXEC statement itself.

    This is real simple -- use parameters, always.  Anything else is potentially vulnerable to SQL injection.  Someone mentioned that the protection could be put in the page by casting the query string parameter as an int, but that misses the point.  Why would you ever create a DB layer method that exposes a potential SQL injection vulnerability when it is so easy to protect against it?

    Please complete the following function so that it contains a sql injection possibility:

    Report[] GetReports(int reportId)

    {

       // Your code here

    }



  • @Sutherlands said:

    @Jaime said:

    @bertram said:

    Do we know for sure that no escaping is happening here? I mean, we can't see inside the sproc, so we're basically guessing? Or is it required to do the escaping outside the sproc? And if so, why?

    Doesn't matter.  You do SQL-injection protection at the point the client makes a database call.  Escaping inside the proc cannot fix the problem that you can inject SQL into the EXEC statement itself.

    This is real simple -- use parameters, always.  Anything else is potentially vulnerable to SQL injection.  Someone mentioned that the protection could be put in the page by casting the query string parameter as an int, but that misses the point.  Why would you ever create a DB layer method that exposes a potential SQL injection vulnerability when it is so easy to protect against it?

    Please complete the following function so that it contains a sql injection possibility:

    Report[ GetReports(int reportId)

    {

       // Your code here

    }

    Um, are you arguing against or for using parameters, or are you saying that the application logic should handle the canonicalization/escaping of all known (and unknown) data types and formats?



  • @C-Octothorpe said:

    Um, are you arguing against or for using parameters, or are you saying that the application logic should handle the canonicalization/escaping of all known (and unknown) data types and formats?

    In this particular instance, I'm not arguing for or against them.  I always use them, though. 

    The only thing I'm arguing against is the idea that if you have a parameter that is passed in from a URL, "there's an obvious SQL insertion security bug", and that without using stored procs there's no way you can create a DB layer that doesn't protect against them. (Especially since the original point already used stored procs, AND had a sql insertion flaw)



  • @Sutherlands said:

    @Jaime said:

    @bertram said:

    Do we know for sure that no escaping is happening here? I mean, we can't see inside the sproc, so we're basically guessing? Or is it required to do the escaping outside the sproc? And if so, why?

    Doesn't matter.  You do SQL-injection protection at the point the client makes a database call.  Escaping inside the proc cannot fix the problem that you can inject SQL into the EXEC statement itself.

    This is real simple -- use parameters, always.  Anything else is potentially vulnerable to SQL injection.  Someone mentioned that the protection could be put in the page by casting the query string parameter as an int, but that misses the point.  Why would you ever create a DB layer method that exposes a potential SQL injection vulnerability when it is so easy to protect against it?

    Please complete the following function so that it contains a sql injection possibility:

    Report[ GetReports(int reportId)

    {

       // Your code here

    }

    Solution A: Review all code to insure that no code path creates a SQL injection vulnerability.  Solution B: Implement a rule that all database calls use parameters, any exceptions must have a justification and go through an additional security review.

    Which solution is more effective?  Which is less effort?



  • @Sutherlands said:

    @C-Octothorpe said:

    Um, are you arguing against or for using parameters, or are you saying that the application logic should handle the canonicalization/escaping of all known (and unknown) data types and formats?

    In this particular instance, I'm not arguing for or against them.  I always use them, though. 

    The only thing I'm arguing against is the idea that if you have a parameter that is passed in from a URL, "there's an obvious SQL insertion security bug", and that without using stored procs there's no way you can create a DB layer that doesn't protect against them. (Especially since the original point already used stored procs, AND had a sql insertion flaw)

    It's a SQL injection vulnerability waiting to happen, kill it before it grows up.  Nobody said stored procedures protect against injection, we all said the parameterization provides the protection.  The argument has little to do with the URL parameter (although that made this one worse) and a lot to do with creating a proper data access layer.



  • @Sutherlands said:

    @C-Octothorpe said:

    Um, are you arguing against or for using parameters, or are you saying that the application logic should handle the canonicalization/escaping of all known (and unknown) data types and formats?

    In this particular instance, I'm not arguing for or against them.  I always use them, though. 

    The only thing I'm arguing against is the idea that if you have a parameter that is passed in from a URL, "there's an obvious SQL insertion security bug", and that without using stored procs there's no way you can create a DB layer that doesn't protect against them. (Especially since the original point already used stored procs, AND had a sql insertion flaw)

    Ahh, OK...  I agree with you to a point, and that is that sanitization should be done at every trust boundary, and in this case, we have two boundaries:

    1) Request URL parameters to application code (sanitize here to "known-good")

    2) Passing parameters to database (sanitize to SQL "known-good" via parameters)

    If we were to pass the argument(s) to LDAP or an XPath query, number 2 would be different for each case.  Everybody needs to sanitize for the current context/use.



  • @Jaime said:

    @Sutherlands said:

    @Jaime said:

    @bertram said:

    Do we know for sure that no escaping is happening here? I mean, we can't see inside the sproc, so we're basically guessing? Or is it required to do the escaping outside the sproc? And if so, why?

    Doesn't matter.  You do SQL-injection protection at the point the client makes a database call.  Escaping inside the proc cannot fix the problem that you can inject SQL into the EXEC statement itself.

    This is real simple -- use parameters, always.  Anything else is potentially vulnerable to SQL injection.  Someone mentioned that the protection could be put in the page by casting the query string parameter as an int, but that misses the point.  Why would you ever create a DB layer method that exposes a potential SQL injection vulnerability when it is so easy to protect against it?

    Please complete the following function so that it contains a sql injection possibility:

    Report[ GetReports(int reportId)

    {

       // Your code here

    }

    Solution A: Review all code to insure that no code path creates a SQL injection vulnerability.  Solution B: Implement a rule that all database calls use parameters, any exceptions must have a justification and go through an additional security review.

    Which solution is more effective?  Which is less effort?

    Solution B.  Did I say something that implied otherwise?


  • @Sutherlands said:

    @Jaime said:

    @Sutherlands said:

    Please complete the following function so that it contains a sql injection possibility:

    Report[ GetReports(int reportId)

    {

       // Your code here

    }

    Solution A: Review all code to insure that no code path creates a SQL injection vulnerability.  Solution B: Implement a rule that all database calls use parameters, any exceptions must have a justification and go through an additional security review.

    Which solution is more effective?  Which is less effort?

    Solution B.  Did I say something that implied otherwise?
    Yes.  You challenged the board to show an injection vulnerability in what looks like non-data layer code.  That would be a unit of work that would only be present in Solution A.  Solution B wouldn't even require looking at non-data layer code.

  • ♿ (Parody)

    @Jaime said:

    Yes.  You challenged the board to show an injection vulnerability in what looks like non-data layer code.  That would be a unit of work that would only be present in Solution A.  Solution B wouldn't even require looking at non-data layer code.

    While I doubt anyone will seriously argue (trolls, flames and pedantic dickweedery aside) with your Solutions A and B, they pretty much ignore what he asked. He asked for a way to create a vulnerability, and you talked about ways to prevent them. Did you imagine that you answered his question?

    My answer would be something along the line that you don't have a classic injection vulnerability, but assuming the int came from the user, you could certainly get buggy behavior by executing the wrong thing, which could be potentially harmful, depending on what possibilities existed to be run, how it was used, etc. Frankly, I don't see how Solution B mitigates that either, assuming that the int still determines which report / sproc / query you call, which was the original scenario.


  • Garbage Person

    @blakeyrat said:

    Oops, not certain about this one without seeing what SQLHelper does.
    SQLHelper, assuming it's the one I think it is, is part of Microsoft's Enterprise Library ( http://msdn.microsoft.com/en-us/library/ff648951.aspx ) - it really should be in the god damned framework in the first place, but it isn't for whatever idiotic reason. SQLHelper.getDataset() wraps all the stupid getDataReader bullshit you'd normally have to jump through to get a god damned DataSet of your query results. Getting a DataSet is in the top 3 things you ever do with a database (the other two being .ExecuteNonQuery and .ExecuteScalar) but they didn't give you a god fucking damned framework class for it.

     

    What the OP's example is doing is just running the assembled query text on the server without going through the parameterization components. There's no stored-procedure-to-get-a-procedure (thank fucking god)



  • @boomzilla said:

    @Jaime said:
    Yes.  You challenged the board to show an injection vulnerability in what looks like non-data layer code.  That would be a unit of work that would only be present in Solution A.  Solution B wouldn't even require looking at non-data layer code.

    While I doubt anyone will seriously argue (trolls, flames and pedantic dickweedery aside) with your Solutions A and B, they pretty much ignore what he asked. He asked for a way to create a vulnerability, and you talked about ways to prevent them. Did you imagine that you answered his question?

    OK, I'll answer the question...

    @Sutherlands said:
    Please complete the following function so that it contains a sql injection possibility:

    Report[]] GetReports(int reportId)

    {

       // Your code here

    }

    <font color="#0000ff" size="2"><font color="#0000ff" size="2">private</font></font><font size="2"> </font><font color="#008080" size="2"><font color="#008080" size="2">Report[]]</font></font><font size="2"> GetReports(</font><font color="#0000ff" size="2"><font color="#0000ff" size="2">int</font></font><font size="2"> reportId)</font><font size="2">

    {

    </font><font color="#008080" size="2"><font color="#008080" size="2">String</font></font><font size="2"> sqlTxt = </font><font color="#800000" size="2"><font color="#800000" size="2">"exec usp_StoredProcName "</font></font><font size="2"> + reportId + <font color="#800000" size="2">",'"</font> + Request.QueryString[<font color="#800000" size="2">"Format"</font>] + <font color="#800000" size="2">"'"</font>;</font><font size="2"> </font>

    <font size="2"></font><font color="#008080" size="2"><font color="#008080" size="2">DataSet</font></font><font size="2"> ds = </font><font color="#0000ff" size="2"><font color="#0000ff" size="2">new</font></font><font size="2"> </font><font color="#008080" size="2"><font color="#008080" size="2">DataSet</font></font><font size="2">();</font>

    <font size="2">ds = </font><font color="#008080" size="2"><font color="#008080" size="2">SqlHelper</font></font><font size="2">.ExecuteDataset(</font><font color="#008080" size="2"><font color="#008080" size="2">GlobalConst</font></font><font size="2">.ConnectionString, System.Data.</font><font color="#008080" size="2"><font color="#008080" size="2">CommandType</font></font><font size="2">.Text, sqlTxt);</font><font size="2"> </font>

    <font size="2"></font><font color="#0000ff" size="2"><font color="#0000ff" size="2">return</font> </font><font size="2"><font color="#008080" size="2">Report</font>.FromTable(ds[0]);</font>

    <font size="2">

    }

    </font>

    I didn't answer the question originally because it's so easy to create a vulnerability that I instead tried to point out an actual solution to the problem. Besides, even if his question was unanswerable, that would only be a single code path that is injection proof.



  • @Sutherlands said:

    Please complete the following function so that it contains a sql injection possibility:

    Report[]] GetReports(int reportId)
    {
        return execute(
            "select foo from bar " +
            "where a < b-" + reportId +
            " and topsecret = 0");
    }
    Disclaimer: I do not have access to a SQL server right now and my experience with SQL is limited, so I do not know if this actually works.


  • @Weng said:

    @blakeyrat said:

    Oops, not certain about this one without seeing what SQLHelper does.
    SQLHelper, assuming it's the one I think it is, is part of Microsoft's Enterprise Library ( http://msdn.microsoft.com/en-us/library/ff648951.aspx ) - it really should be in the god damned framework in the first place, but it isn't for whatever idiotic reason. SQLHelper.getDataset() wraps all the stupid getDataReader bullshit you'd normally have to jump through to get a god damned DataSet of your query results. Getting a DataSet is in the top 3 things you ever do with a database (the other two being .ExecuteNonQuery and .ExecuteScalar) but they didn't give you a god fucking damned framework class for it.

     

    What the OP's example is doing is just running the assembled query text on the server without going through the parameterization components. There's no stored-procedure-to-get-a-procedure (thank fucking god)

    The Enterprise Library encourages building sql as text, and by making ad-hoc sql a one-liner, encourages embedding it in business code. It's really not that hard to build a command properly and doing it properly with the Enterprise Library requires you to build a DBCommand and add parameters to it. By the time you are done, it isn't any less work than native ADO.Net. The Logging Application Block in the Enterprise Library predates TraceSource and friends that were added in .Net 2.0. I find the new native classes to be much better than the Enterprise Library stuff.


  • Garbage Person

     @Jaime said:

    The Enterprise Library encourages building sql as text, and by making ad-hoc sql a one-liner, encourages embedding it in business code. It's really not that hard to build a command properly and doing it properly with the Enterprise Library requires you to build a DBCommand and add parameters to it. By the time you are done, it isn't any less work than native ADO.Net. The Logging Application Block in the Enterprise Library predates TraceSource and friends that were added in .Net 2.0. I find the new native classes to be much better than the Enterprise Library stuff.
    Oh, yeah. It's no fucking good at all in general - but if you don't use anything but sprocs, SqlHelper.executeDataset is made of AWESOME. I mean, otherwise I'd have to look up how to fucking make a dataset from a DataReader every single time I moved to a new project. The rest of the library can go to hell.

     

    Incidentally, you don't need to build a DBCommand, you can just call it straight off and give your params either singly or in an array:

    public DataTable getOrganizationContactList(Int32 orgID)
            {
                return SqlHelper.ExecuteDataset(conn, CommandType.StoredProcedure, "sp_getContactsByOrganization", new SqlParameter("@orgID", orgID)).Tables[0];
            }

     

     

    And you can one-liner inline SQL with the native classes too - 

    new SqlCommand("SQL GOES HERE", conn).ExecuteScalar()



  • @Jaime said:

    Request.QueryString[<font color="#800000" size="2">"Format"</font>]
    lawl

    @fatbull said:

    @Sutherlands said:

    Please complete the following function so that it contains a sql injection possibility:

    Report[] GetReports(int reportId)
    {
        return execute(
            "select foo from bar " +
            "where a < b-" + reportId +
            " and topsecret = 0");
    }

    Disclaimer: I do not have access to a SQL server right now and my experience with SQL is limited, so I do not know if this actually works.

    Hey that's pretty good.  I don't know if it works either.

    @Weng said:

    otherwise I'd have to look up how to fucking make a dataset from a DataReader every single time I moved to a new project
    Or you could just... convert the data to actual objects/storage?



  • @Weng said:

     @Jaime said:

    The Enterprise Library encourages building sql as text, and by making ad-hoc sql a one-liner, encourages embedding it in business code. It's really not that hard to build a command properly and doing it properly with the Enterprise Library requires you to build a DBCommand and add parameters to it. By the time you are done, it isn't any less work than native ADO.Net. The Logging Application Block in the Enterprise Library predates TraceSource and friends that were added in .Net 2.0. I find the new native classes to be much better than the Enterprise Library stuff.
    Oh, yeah. It's no fucking good at all in general - but if you don't use anything but sprocs, SqlHelper.executeDataset is made of AWESOME. I mean, otherwise I'd have to look up how to fucking make a dataset from a DataReader every single time I moved to a new project. The rest of the library can go to hell.

    Why would you ever make a DataSet from a DataReader?  If you want a firehose type cursor, use a DataReader.  If you want to load the data into memory, use a DataSet.  Doing both is pointless.



  • @Jaime said:

    @Weng said:

     @Jaime said:

    The Enterprise Library encourages building sql as text, and by making ad-hoc sql a one-liner, encourages embedding it in business code. It's really not that hard to build a command properly and doing it properly with the Enterprise Library requires you to build a DBCommand and add parameters to it. By the time you are done, it isn't any less work than native ADO.Net. The Logging Application Block in the Enterprise Library predates TraceSource and friends that were added in .Net 2.0. I find the new native classes to be much better than the Enterprise Library stuff.
    Oh, yeah. It's no fucking good at all in general - but if you don't use anything but sprocs, SqlHelper.executeDataset is made of AWESOME. I mean, otherwise I'd have to look up how to fucking make a dataset from a DataReader every single time I moved to a new project. The rest of the library can go to hell.

    Why would you ever make a DataSet from a DataReader?  If you want a firehose type cursor, use a DataReader.  If you want to load the data into memory, use a DataSet.  Doing both is pointless.

    Well, technically a DataSet *is* populated via DataReader under the sheets.</pedant>

    But yes, these two serve completely different purposes and I have traditionally gone the reader route as I usually use POCOs, but DataSets have their place too.  They're perfect for report generation when outputting to a GridView as it really doesn't make much sense to create a custom BO for that single use.



  • @Jaime said:

    Why would you ever make a DataSet from a DataReader?  If you want a firehose type cursor, use a DataReader.  If you want to load the data into memory, use a DataSet.  Doing both is pointless.

    Wasn't that his point?


  • @Sutherlands said:

    @Jaime said:

    Why would you ever make a DataSet from a DataReader?  If you want a firehose type cursor, use a DataReader.  If you want to load the data into memory, use a DataSet.  Doing both is pointless.

    Wasn't that his point?
    No.  His point was that he keeps having to look up how to make a DataSet from a DataReader.  I was pointing out that there is almost zero need to do so.


  • @Jaime said:

    @Sutherlands said:

    @Jaime said:

    Why would you ever make a DataSet from a DataReader?  If you want a firehose type cursor, use a DataReader.  If you want to load the data into memory, use a DataSet.  Doing both is pointless.

    Wasn't that his point?
    No.  His point was that he keeps having to look up how to make a DataSet from a DataReader.  I was pointing out that there is almost zero need to do so.
    No.  His point was that he uses EnterpriseLibrary to get a DataSet instead of a DataReader so that he DOESN'T have to look up how to make a DataSet.  He specifically said he's not using a DataReader to make a DataSet.

  • Garbage Person

    @Jaime said:

    Why would you ever make a DataSet from a DataReader?
    Because that's the only bloody way to make a DataSet without using the Enterprise Library? (Okay. I just looked it up. What I've been calling DataReader here is a DataAdapter. Shows how often I bother because - fuck dude, one line of code that works and does everything it needs to do? And all I need to do is include an obsolete library? Good deal.)

     

    Seriously, who writes back to a database from a dataset?


Log in to reply