Access asking for a query parameter unnecessarily? (Access VBA and SQL)

[patjones]

Hi -

I have a bound form with a bunch of text boxes connected to various fields. When someone types a search value into txtSearch and either tabs out or hits Enter, the following code runs (variable declarations, error handling and clean up omitted):

strGetPersonSQL = "PARAMETERS [LastName] CHAR; " & _
                  "SELECT tblBasicInfo.*," & _
                         "tblReferral.*," & _
                         "tblCountryNames.fldCountry " & _
                  "FROM   tblCountryNames INNER JOIN " & _
                         "(tblBasicInfo LEFT JOIN " & _
                         "tblReferral " & _
                    "ON   tblBasicInfo.fldCaseID=tblReferral.fldCaseID) " & _
                    "ON   tblBasicInfo.fldCountryID=tblCountryNames.fldCountryID " & _
                  "WHERE  tblBasicInfo.fldNameLast=[LastName]"
 
Set qdf = CurrentDb.CreateQueryDef("qryGetPersonFromSearch", strGetPersonSQL)
qdf("LastName") = Me.txtSearch.Value
 
Set rst = qdf.OpenRecordset(dbOpenSnapshot)
 
If rst.EOF Then
    MsgBox "Nothing found. Try again...", vbExclamation + vbOKOnly, "Last Name Lookup"
    rst.Close
    Set rst=Nothing
    Exit Sub
Else
    Me.RecordSource = "qryGetPersonFromSearch"
End If

Things are fine up to where I assign the form's recordsource, at which point Access raises one of those annoying boxes asking for the value of LastName...but I already passed it in via the PARAMETER clause. What is going on here?

As a side note, I recognized that I could assign a SQL string to the recordsource directly, but then I don't know how I would use a parameter. I'm trying to avoid using the text box name directly in the WHERE clause. Thanks.

Pat

[ChipR]

Sorry, I'm not too familiar with querydefs and parameters. The table at http://support.microsoft.com/kb/142938 has Text, but doesn't list CHAR as a variable type, nor does the example use [ ]. Maybe that could be part of the problem?

[NeoPa]

Having reformatted your code to be legible, I wonder what you mean by saying you have already specified the value of [lastName] in your PARAMETER(S) statement. Certainly the type has been specified, but no value is passed.

What are you actually trying to do?

Do you have the value of [LastName] (that you want to use) available to the code at the time the SQL is created?

[patjones]

Hi Chip -

Thanks for responding. I did change the parameter type to TEXT, but it doesn't seem to matter. I figured CHAR was fine because it does actually open the recordset okay with it written out like that.

The brackets don't matter either, apparently. But thanks for the suggestions!

Pat

[patjones]

Hi NeoPa -

Doesn't the line

qdf("LastName") = Me.txtSearch.Value

assign whatever I type into the text box to the parameter? At least this is how I've done it in the past. And, when I test this out with a last name that does not exist in the database the rst.EOF test catches it. So I'm pretty sure the query is getting the value from txtSearch. Thanks for responding.

Pat

[NeoPa]

Very possibly Pat (I overlooked that).

I would consider shoving that value into the SQL directly though :

     "WHERE  tblBasicInfo.fldNameLast='" & Me.txtSearch & "'"

I cannot say why your code is not working, but I expect this version would avoid the problem.

[patjones]

It definitely will avoid the problem, and I must say simplify the code - for then I can eliminate all the parameter stuff. But my concern in doing it the way you're suggesting is SQL injection. So I wanted to use the parameter method because of that.

Is there some way to avoid SQL injection even when inserting the text box value directly into the string like that?

[ADezii]

May be a simple oversight/syntax error on your part, replace Line # 12 with:

qdf.Parameters("LastName").Value = Me.txtSearch.Value 

[patjones]

Hi ADezii -

I have seen that notation before and tried it here, but to no avail. If there is a way that I can prevent SQL injection while embedding Me.txtSearch.Value into the WHERE clause directly, perhaps that is the route I should take. But I will still be bothered as to why the parameter formalism isn't working. Thanks for responding.

Pat

[ADezii]

@zepphead80

1. I have duplicated your functionality and have had no problem with it. If you would like to Upload the Database with some sample data, I pretty sure that I can get the situation resolved one way or another.
2. 
   

   If there is a way that I can prevent SQL injection while embedding Me.txtSearch.Value into the WHERE clause directly

   I think that NeoPa has already suggested this (Post #6), and I feel as though this is an excellent idea.
3. It's you call.

[patjones]

Hi ADezii -

I have attached the file. This is a small, sort of 'test' database for a project. Provided that it gets approval at this point, it will grow considerably - both in number of tables and records. Right now there are just two bogus records in it.

Thanks for your help.

Pat

[ADezii]

@zepphead80
I'll try to look at it this evening. OOPs sorry, I don't have Access 2007. Can you convert it to an earlier Version, say 2002?

[patjones]

Here is 2002-2003. Thanks.

[NeoPa]

@zepphead80
Good thinking.

You can control this on your form with various techniques to ensure there is no quote character in the value. Further info can be found at SQL Injection Attack

[ADezii]

@zepphead80
I've made some substantial Revisions to your Code, while at the same time making the Revised DB available as an Attachment to this Post. I intentionally left Comments outs, so should you have any questions, feel free to ask. A couple of the Major Revisions were:

1. Removed the Code from the context of the Sub-Routine and placed in directly into the Click() Event of cmdLookup.
2. Removed the Call to the Sub-Routine from the AfterUpdate() Event of txtSearch where I thought it was inappropriate.
3. Rather than dealing with the more complex PARAMETERS Collection, I incorporated the Criteria into the WHERE Clause of the SQL Statement. If you remember correctly, this was NeoPa's suggestion.
4. I'll post the Code for reference purposes, but download the Revised Attachment to really see what is going on:
   

   Private Sub cmdLookup_Click()
   On Error Resume Next
   Dim strGetPersonSQL As String
   Dim qdf As DAO.QueryDef
   Dim rst As DAO.Recordset
    
   strGetPersonSQL = "SELECT tblBasicInfo.*, tblReferral.*, tblCountryNames.fldCountry " & _
                     "FROM tblCountryNames INNER JOIN (tblBasicInfo LEFT JOIN tblReferral ON tblBasicInfo.fldCaseID = " & _
                     "tblReferral.fldCaseID) ON tblBasicInfo.fldCountryID = tblCountryNames.fldCountryID WHERE " & _
                     "tblBasicInfo.fldNameLast = '" & Forms!frmMain![txtSearch] & "';"
    
   If IsNull(Me![txtSearch]) Then
     MsgBox "No Last Name to search on!", vbExclamation, "Criteria Missing"
       Exit Sub
   End If
    
   CurrentDb.QueryDefs.Delete ("qryGetPersonFromSearch")
    
   On Error GoTo Err_cmdLookup_Click
    
   Set qdf = CurrentDb.CreateQueryDef("qryGetPersonFromSearch", strGetPersonSQL)
    
   Set rst = qdf.OpenRecordset(dbOpenSnapshot)
    
   If rst.EOF Then
     MsgBox "No Record(s) found for Last Name of [" & Me![txtSearch] & "]." & _
             vbCrLf & vbCrLf & "Try again...", vbExclamation + vbOKOnly, "Last Name Lookup"
   Else
     Me.RecordSource = "qryGetPersonFromSearch"
   End If
    
   Exit_cmdLookup_Click:
     If Not qdf Is Nothing Then
       qdf.Close
       Set qdf = Nothing
     End If
     If Not rst Is Nothing Then
       rst.Close
       Set rst = Nothing
     End If
       Exit Sub
    
   Err_cmdLookup_Click:
     MsgBox Err.Description, vbExclamation, "Error in cmdLookup_Click()"
     Resume Exit_cmdLookup_Click
   End Sub

[NeoPa]

To make this Injection-proof, simply add the following code after line #5 :

Dim strCheck As String
 
If Replace(Replace(Forms!frmMain![txtSearch], _
                   """", _
                   ""), _
           "'", _
           "") <> Forms!frmMain![txtSearch] Then
    'Handle SQL Injection here
End If

PS. Alternatively, you may decide simply to strip out any quote characters and pass that string along, instead of advertising that your code is on to them ;)

[patjones]

Thanks to the both of you for addressing my issue. I will absorb this overnight, and let you know how it works out!

Pat

[ChipR]

An alternative is the Instr function.

If Instr(Forms!frmMain![txtSearch], """") > 0 Or _
   Instr(Forms!frmMain![txtSearch], "'") > 0 Then

[patjones]

Thank you all. Things work quite well.

ADezii -

I put all the code in the Click event for the command button as you suggested, and removed the parameter formalism. I also really like the tests for existence of qdf and rst at the end of the sub and have added those as well.

NeoPa -

I like the Replace method, but am simply assigning those nested Replace statements to a string variable and then putting that into the SQL string rather than doing the test.

I appreciate the time and effort. Thanks so much.

[NeoPa]

It's always a pleasure working with someone who's prepared to get involved Pat. Responding so fully, as you do, keeps everyone in the picture and everyone feeling valued.

BTW I hope you agree with me that ADezii's post #15 is the most apposite for the question (as I've selected it).

[patjones]

ADezii's code is basically what I implemented and consider the best solution.

I'm still not sure why the parameter didn't work though. I usually use unbound forms, pulling the data from the tables with ADO and then assigning text box values item by item referencing the recordset. I've found that ADO makes use of parameters straightfoward.

Thanks!

[ChipR]

I recommend you try using bound forms the next chance you get. Being a programmer before a database administrator, I started with unbound forms and lots of code, but have learned enough to use bound forms. Funny how the "easy" way was not as easy as just writing code. Anyway, the result is much cleaner and faster, and I have redone most of my forms.

[patjones]

Indeed, I am using a bound form for this project...really my first time using one. It does seem to cut down on the code quite a bit. It also makes edits really easy.

Thanks for your insight Chip.