How NOT to pass parameters into a query



  • some lovely code from a "logic" class full of static methods- this one creates a HQL string to pull some aggregate data for a report... manages to not only write unsanitized user input directly into the query but also use both positional and named parameters too. 

     

     

          private static String buildQuery(Form form)<?xml:namespace prefix = o ns = "urn:schemas-microsoft-com:office:office" /><o:p></o:p>      {<o:p></o:p>

                StringBuffer hql = new StringBuffer(BUF_SIZE);

     

                hql.append("FROM SummaryTable e ");<o:p></o:p><o:p> </o:p>            boolean aSpecified = StringHelper.isValid(form.getA())<o:p></o:p>                  && !form.getA().equals(Constants.RPT_ALL_A_OPTION);<o:p></o:p>            boolean bSpecified = StringHelper.isValid(form.getB())<o:p></o:p>                  && !form.getB().equals(Constants.RPT_ALL_B_OPTION);<o:p></o:p><o:p> </o:p>            if (bSpecified && !aSpecified)<o:p></o:p>            {<o:p></o:p>                  hql.append(", SecondTable f ");<o:p></o:p>            }<o:p></o:p>            <o:p></o:p>            hql.append("WHERE e.x = ? AND e.y = ? AND e.z = ? ");<o:p></o:p>            hql.append("AND e.Time BETWEEN :dateFrom AND :dateTo ");<o:p></o:p>            hql.append("AND e.ErrCode not like '%O%' ");<o:p></o:p>            hql.append("AND e.ErrCode is not null ");<o:p></o:p><o:p> </o:p>            if ((form.getC() != null) && !form.getC().equals("0"))<o:p></o:p>            {<o:p></o:p>                  hql.append("AND e.c = '" + form.getC() + "' ");<o:p></o:p>            }<o:p></o:p><o:p> </o:p>            if ((form.getD() != null) && (!form.getD().equals("0")))<o:p></o:p>            {     <o:p></o:p>                  hql.append("AND e.d = '" + form.getD() + "' ");<o:p></o:p>            }<o:p></o:p><o:p> </o:p>            if (StringHelper.isValid(form.getE()))<o:p></o:p>            {     <o:p></o:p>                  hql.append("AND e.E >= " + form.getE() + " ");<o:p></o:p>            }<o:p></o:p>            <o:p></o:p>            if((form.getF() != null) && !form.getF().equals("0"))<o:p></o:p>            {<o:p></o:p>                  hql.append("AND e.F = " + form.getF() + " ");<o:p></o:p>            }<o:p></o:p>            <o:p></o:p>            if (aSpecified)<o:p></o:p>            {<o:p></o:p>                  hql.append("AND e.a = '" + form.getA() + "' ");<o:p></o:p>            }<o:p></o:p>            else if (bSpecified)<o:p></o:p>            {<o:p></o:p>                  hql.append("AND e.g = f.g ");<o:p></o:p>                  hql.append("AND f.b = '" + form.getB() + "' ");<o:p></o:p>            }<o:p></o:p>            <o:p></o:p>

                hql.append("ORDER BY e.d ");

    <o:p> </o:p>            return hql.toString();<o:p></o:p>

          }



  • WTF did you run that through? Just drop it in a PRE block and forget syntax highlighting. Damn!



  •  Dude, someone vomited on your post after eating alphabits.



  • The real WTF is the forum software 

    @jaamkie said:

         

    private static String buildQuery(Form form) {

      StringBuffer hql = new StringBuffer(BUF_SIZE);

      hql.append("FROM SummaryTable e ");

      boolean aSpecified = StringHelper.isValid(form.getA())

        && !form.getA().equals(Constants.RPT_ALL_A_OPTION);

      boolean bSpecified = StringHelper.isValid(form.getB())

        && !form.getB().equals(Constants.RPT_ALL_B_OPTION);

      if (bSpecified && !aSpecified) {

        hql.append(", SecondTable f "); 

      }

      hql.append("WHERE e.x = ? AND e.y = ? AND e.z = ? "); 

      hql.append("AND e.Time BETWEEN :dateFrom AND :dateTo "); 

      hql.append("AND e.ErrCode not like '%O%' ");

      hql.append("AND e.ErrCode is not null ");

      if ((form.getC() != null) && !form.getC().equals("0")) {

        hql.append("AND e.c = '" + form.getC() + "' ");

      }

      if ((form.getD() != null) && (!form.getD().equals("0"))) {

        hql.append("AND e.d = '" + form.getD() + "' ");

      }

      if (StringHelper.isValid(form.getE())) {

        hql.append("AND e.E >= " + form.getE() + " ");

      }

      if((form.getF() != null) && !form.getF().equals("0")) {

        hql.append("AND e.F = " + form.getF() + " ");

      }

      if (aSpecified) { 

        hql.append("AND e.a = '" + form.getA() + "' ");

      }

      else if (bSpecified) {

        hql.append("AND e.g = f.g ");

        hql.append("AND f.b = '" + form.getB() + "' "); 

      }

      hql.append("ORDER BY e.d ");

      return hql.toString();

    }



  • No joins. +1 WTFs. 



  • Actually TRWTF is my latest dev environment setup- have to physically switch networks and static IPs to have test database access vs to check in/out any source if I find that I need to edit a new file...



  • @Benanov said:

    No joins. +1 WTFs. 

    Gah?  It looks like it has a join to me..

     

    Also, WTF is HQL? 



  • @morbiuswilters said:

    Also, WTF is HQL? 

     

    Hibernate query language. GIYF 



  • @ammoQ said:

    Hibernate query language. GIYF 

    GIYF?  What's that?  Let me check Google... ooohhhhh! 



  • @morbiuswilters said:

    @ammoQ said:

    Hibernate query language. GIYF 

    GIYF?  What's that?  Let me check Google... ooohhhhh! 

    Heh, I took it to mean, "Google it, you F***er!" Close!

    `


  • @morbiuswilters said:

    @Benanov said:

    No joins. +1 WTFs. 

    Gah?  It looks like it has a join to me..

     

     

    It works AFAICT (", SecondTable f" is only included under certain conditions, and in those conditions, "e.g = f.g" is also included as part of the WHERE clause) but it's ugly (this should not be under debate) and deprecated ("from e, f where e.g = f.g" is deprecated these days in favor of "from e join f on e.g = f.g").

     



  • @emurphy said:

    It works AFAICT (", SecondTable f" is only included under certain conditions, and in those conditions, "e.g = f.g" is also included as part of the WHERE clause) but it's ugly (this should not be under debate) and deprecated ("from e, f where e.g = f.g" is deprecated these days in favor of "from e join f on e.g = f.g").

    Wouldn't "it's ugly" be a subjective judgement?  It's just an alternative way to state the same thing.



  • sadly I knew a guy (charismatic senior manager, coded exclusively in painfully-procedural PL/SQL, no tests/documentation, production database access to slip in little "fixes" without normal process, forgetful of version control, and excessively busy with multiple projects) who wrote joins exclusively in this form- actually suspect he might've written this query and passed it off to whoever did the parameter craziness.  Perhaps I just should've posted a snip of the logs this beast generates- indignant SQL Warnings and UnprocessedContinuationReference exceptions, plus beauties like this- seeing "WHERE foo = ? AND bar BETWEEN :dateFrom AND :dateTo AND fizz < 3 AND buzz IS NOT NULL" did elicit a "WTF?" before I grew accustomed to it.



  • @jaamkie said:

    Actually TRWTF is my latest dev environment setup- have to physically switch networks and static IPs to have test database access vs to check in/out any source if I find that I need to edit a new file...

     

     

    Pshh thats nothing. Every time I need to work on a new webapp I have to enter in the connection string and some other settings into my registry. Yes we still use the web.config file too, and the settings in web.config are overriden by a second config file all the while the registry overrides the second config file.




  • I think it's called community server.



  • @emurphy said:

    @morbiuswilters said:

    @Benanov said:

    No joins. +1 WTFs. 

    Gah?  It looks like it has a join to me..

     

     It works AFAICT (", SecondTable f" is only included under certain conditions, and in those conditions, "e.g = f.g" is also included as part of the WHERE clause) but it's ugly (this should not be under debate) and deprecated ("from e, f where e.g = f.g" is deprecated these days in favor of "from e join f on e.g = f.g").

     

    There are no explicit joins. There's this idea of where e.g = f.g but that's not the join syntax that's so popular amongst the kids these days. I would also assume that that doesn't allow the server to optimize properly, or at least makes it a hell of a lot harder to do so.  I really do not know, but as far as I'm concerned, joins are GOOD.

    They also encourage one to explicitly enumerate the relationship between the two tables, which does help to align what the hell one is doing firmly in one's head.



  • @Benanov said:

    There are no explicit joins. There's this idea of where e.g = f.g but that's not the join syntax that's so popular amongst the kids these days. I would also assume that that doesn't allow the server to optimize properly, or at least makes it a hell of a lot harder to do so.  I really do not know, but as far as I'm concerned, joins are GOOD.

    They also encourage one to explicitly enumerate the relationship between the two tables, which does help to align what the hell one is doing firmly in one's head.

    I can't comment on all database servers, but most just treat the implicit joins as inner joins and optimize accordingly.  I don't believe there should be any difference between an implicit or explicit join in terms of performance.



  • @jaamkie said:

    some lovely code from a "logic" class full of static methods- this one creates a HQL string to pull some aggregate data for a report... manages to not only write unsanitized user input directly into the query but also use both positional and named parameters too. 

     

     

          private static String buildQuery(Form form)      {

                StringBuffer hql = new StringBuffer(BUF_SIZE);

     

                hql.append("FROM SummaryTable e ");             boolean aSpecified = StringHelper.isValid(form.getA())                  && !form.getA().equals(Constants.RPT_ALL_A_OPTION);            boolean bSpecified = StringHelper.isValid(form.getB())                  && !form.getB().equals(Constants.RPT_ALL_B_OPTION);             if (bSpecified && !aSpecified)            {                  hql.append(", SecondTable f ");            }                        hql.append("WHERE e.x = ? AND e.y = ? AND e.z = ? ");            hql.append("AND e.Time BETWEEN :dateFrom AND :dateTo ");            hql.append("AND e.ErrCode not like '%O%' ");            hql.append("AND e.ErrCode is not null ");             if ((form.getC() != null) && !form.getC().equals("0"))            {                  hql.append("AND e.c = '" + form.getC() + "' ");            }             if ((form.getD() != null) && (!form.getD().equals("0")))            {                       hql.append("AND e.d = '" + form.getD() + "' ");            }             if (StringHelper.isValid(form.getE()))            {                       hql.append("AND e.E >= " + form.getE() + " ");            }                        if((form.getF() != null) && !form.getF().equals("0"))            {                  hql.append("AND e.F = " + form.getF() + " ");            }                        if (aSpecified)            {                  hql.append("AND e.a = '" + form.getA() + "' ");            }            else if (bSpecified)            {                  hql.append("AND e.g = f.g ");                  hql.append("AND f.b = '" + form.getB() + "' ");            }           

                hql.append("ORDER BY e.d ");

                 return hql.toString();

          }

     

     

    Is this one of those magic-eye pictures? Oh! I see it! It's a kitty cat! 



  • @ShaggyB said:

    @jaamkie said:

    Actually TRWTF is my latest dev environment setup- have to physically switch networks and static IPs to have test database access vs to check in/out any source if I find that I need to edit a new file...

     

     

    Pshh thats nothing. Every time I need to work on a new webapp I have to enter in the connection string and some other settings into my registry. Yes we still use the web.config file too, and the settings in web.config are overriden by a second config file all the while the registry overrides the second config file.


     

    I forgot to mention- I'm a female developer who likes to wear tight skirts and heels, sitting at the front of a rather long row of desks- so kneeling and ducking under my desk every few minutes to switch ports can look a bit inappropriate when managers/clients are walking back and forth behind me...



  • @jaamkie said:

    I forgot to mention- I'm a female developer who likes to wear tight skirts and heels, sitting at the front of a rather long row of desks- so kneeling and ducking under my desk every few minutes to switch ports can look a bit inappropriate when managers/clients are walking back and forth behind me...

    Pics or it didn't happen.



  • @jaamkie said:

    barf 

     

    I thought it was all ugly and unorganized because it was the way it was presented to you, but it was just the forum messing around? 



  • Let me guess -- TRWTF is concatenating HQL rather than using a Criteria query?



  • @morbiuswilters said:

    @jaamkie said:

    I forgot to mention- I'm a female developer who likes to wear tight skirts and heels, sitting at the front of a rather long row of desks- so kneeling and ducking under my desk every few minutes to switch ports can look a bit inappropriate when managers/clients are walking back and forth behind me...

    Pics or it didn't happen.

    Seconded.


  • @jaamkie said:

    I forgot to mention- I'm a female developer who likes to wear tight
    skirts and heels, sitting at the front of a rather long row of
    desks- so kneeling and ducking under my desk every few minutes to
    switch ports can look a bit inappropriate when managers/clients are
    walking back and forth behind me...

    Solution: Wear less^M^M^M^Mmore office appropirate clothing?



  • @Benanov said:

    There are no explicit joins. There's this idea of where e.g = f.g but that's not the join syntax that's so popular amongst the kids these days. I would also assume that that doesn't allow the server to optimize properly, or at least makes it a hell of a lot harder to do so.

    A database system that cannot optimize queries with implicitely written joins is not worth its money.

    I really do not know, but as far as I'm concerned, joins are GOOD.

    They also encourage one to explicitly enumerate the relationship between the two tables, which does help to align what the hell one is doing firmly in one's head.

     

    I wouldn't say explicit joins are bad, but at least I can say I hardly ever use them, and my coworkers also hardly ever use them. We all use implicit joins. Probably a matter of habits.


Log in to reply