• fred (unregistered)

    i don't know VB (which is what i think this is...)  but isn't this only going to fix the first instance of the quote? 

  • CodeJudge (unregistered)

    I find the author guilty of excessive WTFage and sentence him to 30 years of writing unit tests for his code. pounds gavel

  • (cs)

    Wow! you would have to call that function from a loop or something to fix a string with more than one single-quote character in it.  As it is, it only replaces the first single-quote it finds.


    I'm not too good at VB, but isn't there a Replace() function for such things?


        Replace(sName, "'", "`")

        -dZ.

  • (cs)

    Yeah, whereTF is the loop?

    and what value is intPos?

    I'm not VB6 guy but isn't there a Repleace built-in?

  • (cs)

    Ahh, the good ol' days, when VB5 didn't have a Replace() function.

    Where to Start?

    1) I suppose there's no need to check for a second single quote. No way would any one malicious not try that.

    2) I hope (maybe not) there is a complementary function that "unescapes" the quotes

    3) I believe SQL Server can use escaped quotes by inserting a doubleed quote character '' (that's two single quotes)

    4) Not relavent, but know what you're talking about. A grave accent is not an apostrophe, nor will it ever be.

    5) iLen is never used.

    6) This is the best darned function to garble a string ever. (not intPos in the statement that replaces the apostrophe) .. Oh, yes, thats right I forgot to mention ' is an apostrophe. ` is the grave accent. silly me.

    This code should be chucked in the garbage bin, along with its creator.

     

  • cdog (unregistered)
    1. Function name in comments is totally different than the actual function name.
    2. Horrible function name.
    3. sqlChkName2 implies the existence of sqlChkName1 (shudders when thinking about the ramifications of this)
    4. Replacing a special character with a completely different character instead of escaping it.
    5. Not using Replace.
    6. Only replacing the first instance of the single quote.
  • cdog (unregistered) in reply to Mike R

    I hope (maybe not) there is a complementary function that "unescapes" the quotes


    I hope not, because then all the actual poor defenseless grave accents would be senselessly converted to single quotes.
  • wrexen (unregistered) in reply to cdog

    I would guess that the predecessor to this function looks something like this?

    Function sqlChkName1(sName As String) As String
      Dim nCount As Integer
      Do While sName <> sqlChkName2(sName)
        sName = sqlChkName1(sName)
      Loop
      sqlChkName1 = sName
    End Function

  • (cs) in reply to cdog
    1. Probably calling sqlChkName1 for the first quote and sqlChkName2 for the 2nd quote.

    Anonymous:
    1) Function name in comments is totally different than the actual function name.
    2) Horrible function name.
    3) sqlChkName2 implies the existence of sqlChkName1 (*shudders when thinking about the ramifications of this*)
    4) Replacing a special character with a completely different character instead of escaping it.
    5) Not using Replace.
    6) Only replacing the first instance of the single quote.
  • Strikes (unregistered)

    1) The whole thing could be replaced with sNewName = Replace( sName, "'", "`" )

    2) iLen is declared and set, but then never used

    3) intPos is never declared so will probably have a value of 0 when it is used in the second mid statement

    4) Using this code will mean you end up with slanty quotes when the user entered regular quotes, so unless there is matching "sqlUnChkName2" that changes slanty to regular on the way back out what they entered ain't what they'd get.  (and if there WAS a  "sqlUnChkName2" how would it know which slanty quotes were ACTUAL slanty quotes, and which were ones entered by the user)

    5) Now that I type it, the name of the function itself is quite a WTF.

    6) As mentioned previously, this will only replace the first quote, and so is useless on strings that contain more than one.  I can only assume that there is some validation previous to this that ensures that no more than one apostrophe may occur in any string headed for the database.... (I actually worked on a system once where the developer got around the tricky problem of single quotes by disallowing their use - any text fields on the form had javascript validation on them that popped up a "Sorry, single quotes are not supported by this system" whenever you tried to enter a string with a quote in it.  Abigail O'Conner didn't like that system much at the time I picked it up...)

    That's 6 I think!

  • Strikes (unregistered)

    Man, I knew someone would do a list in the time it took me to post that, but didn't expect it to be two people...

  • (cs)

    A slight optimisation here (if you insist on NOT using Replace() for some damn reason):

    sNewName = Left(sName, iPos - 1) + "`" + Right(sName, iLen - iPos - 1)

    As a lucky bonus it would get rid of the unused iLen WTF and has zen-like symmetry :)
  • Fregas (unregistered)

    Hahahahahaha!!!!

    I've done that before, or something quite similar.  My very first web development/programming job was in Coldfusion, and I needed to strip a bunch of "bad" characters, like "'?!&#$%* and so forth.  So I made my own replace function (actually custom tag, since Coldfusion didn't allow user defined functions in those days) that would replace those characters with an empty string.  I didn't realized that coldfusion already had a Replace() function.

    LAME!

  • Dan Hulton (unregistered) in reply to wrexen
    Anonymous:

    I would guess that the predecessor to this function looks something like this?

    Function sqlChkName1(sName As String) As String
      Dim nCount As Integer
      Do While sName <> sqlChkName2(sName)
        sName = sqlChkName1(sName)
      Loop
      sqlChkName1 = sName
    End Function

    I was going to try to be funny and say that your code looked too good to simply be a WTF on the same level as the first WTF, and that you ought to define a spare variable that was never used...

    ...but you beat me to it.

  • (cs) in reply to Charles Nadolski
    Charles Nadolski:
    A slight optimisation here (if you insist on NOT using Replace() for some damn reason):
    sNewName = Left(sName, iPos - 1) + "`" + Right(sName, iLen - iPos - 1)

    As a lucky bonus it would get rid of the unused iLen WTF and has zen-like symmetry :)



    Mm. But not as nice as:

    Mid(sName, iPos, 1) = "`"
  • (cs) in reply to Mike R
    Mike R:
    Charles Nadolski:
    A slight optimisation here (if you insist on NOT using Replace() for some damn reason):
    sNewName = Left(sName, iPos - 1) + "`" + Right(sName, iLen - iPos - 1)

    As a lucky bonus it would get rid of the unused iLen WTF and has zen-like symmetry :)



    Mm. But not as nice as:

    Mid(sName, iPos, 1) = "`"


    Yeah, genius, but it has no zen-like symmetry >:-(
  • cdog (unregistered) in reply to Charles Nadolski
    Charles Nadolski:
    Mike R:
    Charles Nadolski:
    A slight optimisation here (if you insist on NOT using Replace() for some damn reason):
    sNewName = Left(sName, iPos - 1) + "`" + Right(sName, iLen - iPos - 1)

    As a lucky bonus it would get rid of the unused iLen WTF and has zen-like symmetry :)



    Mm. But not as nice as:

    Mid(sName, iPos, 1) = "`"


    Yeah, genius, but it has no zen-like symmetry >:-(


    But it does have a more focused chi, plus its chakras are aligned.
  • (cs) in reply to cdog
    Anonymous:

    But it does have a more focused chi, plus its chakras are aligned.


    Netx thing you know we'll be discussing how to properly implemet Feng Shui in our code

    .  .  .  Oh, wait...
  • (cs) in reply to cdog
    Anonymous:

    But it does have a more focused chi, plus its chakras are aligned.



    Netx thing you know we'll be discussing how to properly implemet Feng Shui in our code


    .  .  .  Oh, wait...

  • (cs) in reply to Mike R

    Whee. My first double-post.

    It would be nice if, when I post a message, and nothing actually goes wrong to, like, not say something went wrong.

  • (cs)

    1. This might be my own ignorance, but why replace an apostrophe ' with a tick ` to make a sql string safe, instead of escaping it with \' ? Escaping it is standard, and you can then unescape all escaped chars in bulk after you retrieve the value, instead of having a special case for apostrophes.

    2. As most folks noted, this function only replaces one apostrophe. Let's (oops, Let`s) fix it... with recursion! (I'm not a VB programmer so forgive me if the syntax is wrong):

    <FONT face="Courier New"><FONT size=2>' __ quotCheck2 __
' Replaces single quote chars (') with apostrophes (`)
'  to get SQL-Safe Strings
Function sqlChkName2(sName As String) As String
  Dim sNewName As String
  Dim iPos As Integer
  Dim iLen As Integer

    iPos = InStr(1,sName,"'")</FONT></FONT>
    <FONT face="Courier New"><FONT size=2>  iLen = Len(sName)
    

    sNewName = sName
If iPos > 0 Then
<FONT color=#ff1493>    sNewName = Mid(sName, 1, iPos - 1) + "`" + sqlChkName2(Mid(sName, iPos + 1))</FONT></FONT></FONT>
    <FONT face="Courier New" color=#ff1493 size=2>  else</FONT>
    <FONT face="Courier New"><FONT size=2><FONT color=#ff1493>    sqlChkName2 = sName  </FONT><FONT style="BACKGROUND-COLOR: #d3d3d3" color=#008000>' (and return from the function here)</FONT></FONT></FONT>
    <FONT face="Courier New"><FONT size=2>  End If
    

    sqlChkName2 = sNewName
End Function</FONT></FONT>

  • Justin Sippel (unregistered)

    I think the ` character is called a backtick, not apostrophe. That's the real WTF here.

    Okay, maybe not.

  • (cs)

    I wrote my own Replace() function in VB once upon a time because I didn't know about the existence of the built-in one. I submitted it to Alex (it was about 10-12 lines of code) but I guess the fact that it worked meant that it wasn't a good enough WTF.

    I think the biggest WTF about this code is that it won't work at all. Ignore the fact that the author made up that "intPos" variable which will end up making the name completely garbled. You're replacing one character with a totally separate character. As far as I know SQL will treat them as different characters, so you're guaranteed to get a string that will technically work, but it won't be the string you want. Way to go genius.

  • (cs) in reply to Manni

    oh! a big WTF!, he forget the Recursive Call!! hohoho

  • (cs)

    #8 - not using parameterised queries in the first place. See Colin Angus Mackay's SQL Injection Attacks article.

  • VB Guy (unregistered) in reply to Charles Nadolski 
    Here goes a nice, one-liner VB6 trick to replace the whole function
    'Dim sName As String
    'sName = "O'Connor"
    If Instr(sName, "'") Then Mid(sName, 1, Instr(sName, "'") = "`"


    'sName now contains "O`Connor"
  • (cs) in reply to VB Guy

    Except your trick only replaces the first apostrophe AND needlessly calls the Instr function twice.

  • (cs) in reply to VB Guy
    Anonymous:
    Here goes a nice, one-liner VB6 trick to replace the whole function
    'Dim sName As String
    'sName = "O'Connor"
    If Instr(sName, "'") Then Mid(sName, 1, Instr(sName, "'") = "`"


    'sName now contains "O`Connor"


    Remind me to avoid self-professed 'VB Guy's in the future.

    So, now. If I enter my name in the web form as O'Shit'; delete from users -- It won't cause problems? Where's your database-driven webpage.. I have some micheif to unleash.

  • (cs) in reply to Mike R

    <font size="2">Where's your database-driven webpage.. I have some micheif to unleash.
         </font>

    can I have some micheif too? I heard its pretty tasty!
         -dZ.


  • VB Guy (unregistered) in reply to rogthefrog

    Right, and that's what I said the function did: "Replace the whole function", which has the same bug ;)

     

  • VB Guy (unregistered) in reply to VB Guy

    Before more people flame against poor me, the paramaters in the Mid call were incorrect :

    If Instr(sName, "'") Then Mid(sName, Instr(sName, "'", 1) = "`"

    Sorry if my post hurts your feelings, I was just trying to perfectly the given function, not trying to fix all the issues it has. :)

     

  • njkayaker (unregistered) in reply to VB Guy

    All of the other WTFs pale before this one: it CHANGES the data! (And it doesn't need to.)

  • VB Guy (unregistered) in reply to VB Guy

    LOL, I'm just killing myself here :D Still wrong parameters and parenthesis number

    If Instr(sName, "'") Then Mid(sName, Instr(sName, "'" ),  1) = "`"

  • Diego Mijelshon (unregistered) in reply to Mike Dimmick
    Mike Dimmick:
    #8 - not using parameterised queries in the first place. See Colin Angus Mackay's SQL Injection Attacks article.


    I think this is the biggest WTF here... he is trying to fix a problem that wouldn't even exist if he did the things the Right Way.

    Of course, everything said is valid (' is different from `, he could use double '', replaces only first, dumb use of Mid, unnecesary iLen, could use Replace instead...)
  • (cs) in reply to VB Guy

    Anonymous:
    If Instr(sName, "'") Then Mid(sName, Instr(sName, "'" ),  1) = "`"
    Great googly moogly.

    I was all ready to reply to your post with a "you can't use MID that way" (setting the MID of a string to a value), but figured I ought to try it first. It works!

    Wonder why I never knew that. Note, though, that you can't do the same with Left or Right:

    <BAD code><bad code>
    Left(myString, 1) = "Z"
    Right(myString, 1) = "X"
    </bad code>

  • mkbosmans (unregistered) in reply to BradC
    BradC:

    Anonymous:
    If Instr(sName, "'") Then Mid(sName, Instr(sName, "'" ),  1) = "`"
    Great googly moogly.

    I was all ready to reply to your post with a "you can't use MID that way" (setting the MID of a string to a value), but figured I ought to try it first. It works!

    Wonder why I never knew that. Note, though, that you can't do the same with Left or Right:

    Left(myString, 1) = "Z"
    Right(myString, 1) = "X"

    </BAD code>

    RTFM, Mid is, besides a function (like Left and Right), also a statement in VB.
    This is really a WTF of VB. Why on earth did the guys at microsoft think `hey, let's make a new statement, complementary to an existing function'.

    <FONT face="Courier New" size=2>sNewName = sName</FONT>

    At least the author of this WTF used a new variable to hold the new value, which saved him from typing ByVal in front of the parameter declaration. O wait, that's standard behaviour in VB6

  • nobody's home (unregistered) in reply to BradC

    Remarkable how many VB'ers apparently don't know about Mid. It's the most efficient way to modify strings in VB IF you use the string version, Mid$. If you don't include the $ then the text gets cast to Variant and then back to String, which looses a bunch of your efficiency. (But not all. Mid, the variant version, is still the best option in VBScript.)

    If trying Left(myString, 1) = "Z", use Mid$(myString, 1, 1) = "Z", of course.

    Ian.

  • Steven Padfield (unregistered)
    1. Function comment has wrong function name
    2. ` is a grave accent, not an apostrophe
    3. The purpose of the function is incorrect: there is a well-defined way to properly escape single-quotes in SQL
    4. Bad function name
    5. The whole function could be replaced by a single call to Replace()
    6. iLen is declared and modified, but never accessed
    7. intPos is never declared, but is accessed
    8. The function only performs a single replacement (and then obliterates the remaining portion of the string, which is probably why the author didn't see the need to make it a loop)

    Here is a "fixed" version:

    <FONT size=4></FONT> 

    <FONT size=4>' __ quotCheck2 __
' Replaces single quote chars (') with apostrophes (`)
'  to get SQL-Safe Strings
Function sqlChkName2(sName As String) As String
  Dim sNewName As String
  Dim iPos As Integer
  Dim iLen As Integer

    iPos = InStr(1,sName,"'")
iLen = Len(sName)
    

    sNewName = sName
  ' recurse to make sure we get them all
If iPos > 0 Then
sNewName = sqlChkName2(Mid(sName, 1, iPos - 1)) + "`" + sqlChkName2(Mid(sName, iPos + 1))
End If
    

    sqlChkName2 = sNewName
End Function</FONT>

  • (cs) in reply to nobody's home
    Anonymous:

    Remarkable how many VB'ers apparently don't know about Mid. It's the most efficient way to modify strings in VB IF you use the string version, Mid$. If you don't include the $ then the text gets cast to Variant and then back to String, which looses a bunch of your efficiency. (But not all. Mid, the variant version, is still the best option in VBScript.)

    If trying Left(myString, 1) = "Z", use Mid$(myString, 1, 1) = "Z", of course.

    Ian.

    or Left$(mystring,1)

    Whenver I use VB, I know I should use the $ on almost all string functions, but I just can't do it .. I feel like I'm writing BASIC code on my old commodore 64 ! 

    As mentioned, VB before version 6.0 did not have a replace function, so you'd write it yourself.

    Did anyone mention that you should NEVER have to manually replace quotes or escape characters in any SQL statement yet?  Even if you want to write ad-hoc SQL instead of using stored procs (or if they aren't available for your RDMS), you should always use parameters.  No escaping, no sql injection, and always more efficient.  Even good old DAO supported this.

  • (cs) in reply to Jeff S

    Oh, and by the way, for the hard core VB programmers out there: 

    This is a perfect example of  why you should ALWAYS use Option Explicit.

  • Gary and the Samoyeds (unregistered) in reply to Jeff S

    You never know; intPos might be a global.

  • (cs) in reply to Jeff S

    I'm the AWATS who submitted this WTF. Just some things I wanted to note:

    - This function is not being called in a loop. It's just

    <FONT face="Courier New">sNewFirstName = chkQuotes2(sFirstName)</FONT>

    and then sNewFirstName is plugged into the SQL statement

    - I have no idea what the ` thingee is called, but the code calls it an "apostrophe" in the comments.

    - Yes, this does mean that anybody with the last name O'Brien becomes O`Brien in our system.

    - The code also contains another function that's exactly the same as sqlChkName2, except the ` has been changed to two single quotes. The name of this function? sqlChkName.

    - Despite the new and improved sqlChkName, both sqlChkName and sqlChkName2 are being called interchangeably throughout the code.

    - sqlChkName2 is generally being called to escape first and last names, while sqlChkName is being called to escape email addresses. Yes, it's perfectly legal (though kind of a WTF) to have a single quote in an email address. I'm guessing sqlChkName came about when someone realized some`[email protected] wasn't quite the same address as some'[email protected].

    I still think the biggest WTF is generating a bunch of INSERTs instead of doing a BCP.

     

     

  • (cs) in reply to Gonzalo
    Gonzalo:
    oh! a big WTF!, he forget the Recursive Call!! hohoho


    In our office, we can call it a big "Robism"...  :D

  • (cs) in reply to A Wizard A True Star

    The function's not quite as funny as all those PHP sites out there that neglect stripslashes() and end up with lots of unreadable ' quotes. Or worse, just stripping everything from the first quote on.

  • joejoe (unregistered)

    I've seen this a lot with older vb projects that are moved to vb6 - yes, this version is still hopelessly broken as a replacement and is still a WTF.

  • Zatanix (unregistered) in reply to joejoe

    What a strange function.. Questions like "Is it supposed to do what it does or is it just totally broken?" pop into mind.. This has definately made my morning more confusing than it had to be. Damn.. now i'll be like this for the rest of the day :(

  • Spidey (unregistered) in reply to Zatanix

    The title should be Replace("Bad programmer", "Bad", "Good") , surely?

  • Aristotle (unregistered)

    I lost interest before I even got from the comment to the code. Mangling data? The whole thing is a WTF in and of itself: written by someone who never heard of escaping. Or placeholders. I mean, the database driver might just know what it needs to escape and how, making sure you cannot forget any special characters and need not maintain any extra code. But that would be too sensible and easy, no?

  • (cs) in reply to rogthefrog
    rogthefrog:
    Except your trick only replaces the first apostrophe AND needlessly calls the Instr function twice.
    Yeah?

    It is supposed to replace the shown function which had the same bug ... behaviour.

    As for the double calling of Instr - that happens only if there is an apostrophe in the name anyway.

  • (cs) in reply to nobody's home
    Anonymous:

    Remarkable how many VB'ers apparently don't know about Mid. It's the most efficient way to modify strings in VB IF you use the string version, Mid$. If you don't include the $ then the text gets cast to Variant and then back to String, which looses a bunch of your efficiency. (But not all. Mid, the variant version, is still the best option in VBScript.)

    If trying Left(myString, 1) = "Z", use Mid$(myString, 1, 1) = "Z", of course.

    Ian.

    Uhm, there is a little problem with this...

    You cannot use Mid$(mystring, 1,1) = "Morethanonecharacter"  if I remember correctly. It only copies as many characters as you define in your Mid$ (iirc). So you cannot use it to escape any characters.

    Drak

Leave a comment on “Replace(&quot;bad programmer&quot;, &quot;good programmer&quot;) ”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article