Parameterized SQL!


  • Garbage Person

    I can't figure out how to do a proper syntax-highlighted code block, so whatever:

     private  SqlCommand BuildDoThingCommand(SqlConnection conn)
            {
                SqlCommand DoThingCommand = conn.CreateCommand();
                DoThingCommand.CommandTimeout = ConfigHelper.GetAppSetting<int>("CommandTimeout"); 
                DoThingCommand.CommandType = CommandType.StoredProcedure;
                DoThingCommand.CommandText = ConfigHelper.GetAppSetting("DoThing"); 
                foreach (string parameter in ConfigHelper.GetAppSetting("DoThingParameters").Split(';'))
                {
                    string[] paramParts = parameter.Split(',');
                    if (paramParts.Length == 2)
                        DoThingCommand.Parameters.Add(paramParts[0], (SqlDbType)Enum.Parse(typeof(SqlDbType), paramParts[1]));
                    else if (paramParts.Length == 3)
                        DoThingCommand.Parameters.Add(paramParts[0], (SqlDbType)Enum.Parse(typeof(SqlDbType), paramParts[1]), int.Parse(paramParts[2]));
                    else
                        throw new Exception("Invalid parameter configuration for 'DoThingParameters'");
                }
                return DoThingCommand;
            
    

    Because there's ZERO chance that could ever go wrong. Protip: It went wrong. That's how I found it.


  • Java Dev

    @Weng said:

    I can't figure out how to do a proper syntax-highlighted code block

    triple backtick.

    What's the point of doing a parametrized query, if the SQL and the parameters come from the same source?


  • Garbage Person

    Note it's only getting the names and types of the parameters. The values come elsewhere. And are hardcoded to array positions. So the "configurable" parameters gain nothing.



  • Hmm, it calls a stored procedure with name and arguments defined in settings... Sorry I don't see the WTF. I can imagine a situation where this could be useful.

    EDIT: OK, if values are hardcoded positionally, then this is not needed. But you didn't show that part.


  • Garbage Person

    The place where the procedure was used did something like this:

    cmd.Parameters[0].Value = ThingA
    cmd.Parameters[1].Value = ThingB
    cmd.Parameters[2].Value = ThingC

    I don't think any sane universe exists in which parameters will occur in a fixed order, but with variable names and types, and with varying procedure names.

    Also, the number of parameters it was trying to redefine mismatched the number in the config.



  • Yeah if this part was also configurable, then it might have made sense.

    It's a WTF.



  • Fuck that? How stupid? What kind of moron made that stupid pattern?

    This reminds me of certain pattern my team decided to use:

    function login(params){
       doLogin(params.username, params.password);
    }
    

    Funny that three-back-ticks works for code blocks, but three-back-ticks + space breaks the whole thing



  • I see code like this, and I don't even analyze the lines. I can just look at the block as a whole, and see that it's bad. SQL injection, inner-platform effect, blonde, brunette, redhead...


Log in to reply