views:

219

answers:

2

I am constructing a search function in a class to be used by several of our asp pages. The idea is simple, take a search term from the user and query the database for the item. Currently I am doing this the wrong way, which is vulnerable to SQL injection attacks (and ELMAH is in there to save the day if something goes wrong):

Public Shared Function SearchByName(ByVal searchterm As String) As DataTable
    SearchByName = New DataTable

    Dim con As New OracleConnection(System.Configuration.ConfigurationManager.ConnectionStrings("OracleDB").ConnectionString)



    Try
        con.Open()
        Dim SqlStr As String = "select ID_ELEMENT, ELEMENT_NAME from table_of_elements where upper(ELEMENT_NAME) like upper('%" & searchterm & "%')"
        Dim cmd As New OracleCommand(SqlStr, con)
        SearchByName.Load(cmd.ExecuteReader)





    Catch ex As Exception
        Elmah.ErrorSignal.FromCurrentContext().Raise(ex)

    End Try
    con.Close()
    con.Dispose()




    Return SearchByName
End Function

String concatenation is BAD. Next thing you know, Bobby Tables wrecks my system. Now, the correct way to do this is to to make a proper oracle variable, by putting :searchterm in the string and adding the following line:

cmd.Parameters.Add(New OracleParameter("SEARCHTERM", searchterm))

The problem is since I am using a like statement, I need to be able to have % on either side of the search word, and I can't seem to do that with '%:searchterm%', it just gives an error of ORA-01036: illegal variable name/number.

Can I parameterize but still have my flexible like statement be a part of it?

+3  A: 

Instead of doing the concatenation in your VB code, do the concatenation in the SQL statement. Then what you're trying to do should work. Here's some SQL illustrating what I'm talking about:

select ID_ELEMENT, ELEMENT_NAME 
from table_of_elements 
where upper(ELEMENT_NAME) like ('%' || upper(:searchterm) || '%')

BTW, you might end up with more efficient queries if you switch the collaction on ELEMENT_NAME to case-insensitive and then remove the calls to upper().

Justin Grant
I'm going to test this real quick. If this works, you'll be the happy owner of 100 shiny new rep.
MadMAxJr
cool! glad to help.
Justin Grant
+1  A: 

Since you're using oracle, another option would be to use Oracle Text to perform the search.

It can take a bit to set up properly, but if you have a large amount of text to search, or have some sort of structured data, it can offer you many more options than a simple wild-card comparison.

It also has some nice features for dealing with multiple languages, if you happen to have that problem as well.

chris
My problem is a bit more narrow in scope than what your link is meant for, but it makes for some very interesting reading, and I wanted to thank you for that.
MadMAxJr