Using dynamic variables in query

Question

I have a page that has an variable number of input fields (this number of fields is saved in the database). Upon submit, the information in these fields is to be saved into a database. The names of the fields are looped through and the current loop number is attached to the end of the field. I then save this into a variable and use the variable in the query. It's getting the correct information from the variable, but isn't actually evaluating the form fields. For instance, the values put into the form fields are:

"#FORM.TEST_LOCATION_1#=two"<br>
"#FORM.TEST_LOCATION_2#=four"<br>
"#FORM.TEST_LOCATION_3#=six"<br>
"#FORM.TEST_LOCATION_4#=eight"<br>
"#FORM.TEST_LOCATION_5#=ten"<br>
"#FORM.TEST_NAME_1#=one"<br>
"#FORM.TEST_NAME_2#=three"<br>
"#FORM.TEST_NAME_3#=five"<br>
"#FORM.TEST_NAME_4#=seven"<br>
"#FORM.TEST_NAME_5#=nine"<br>

The variable that is used in the query gets:

test_location_1 = '#form.test_location_1#', test_name_1 = '#form.test_name_1#', test_location_2 = '#form.test_location_2#', test_name_2 = '#form.test_name_2#', test_location_3 = '#form.test_location_3#', test_name_3 = '#form.test_name_3#', test_location_4 = '#form.test_location_4#', test_name_4 = '#form.test_name_4#', test_location_5 = '#form.test_location_5#', test_name_5 = '#form.test_name_5#',

but instead of putting the values actually entered in the input field in to database, it puts:

"#form.test_location_1#"
"#form.test_location_2#"
"#form.test_location_3#"
"#form.test_name_1#"
"#form.test_name_2#"
"#form.test_name_3#"
etc

The code I'm using right now is:

 <cfset session.updque = ''>
    <cfloop from="1" to="#session.test_numfields#" index="numf">
      <cfset session.updque &= "test_location_" & #numf# &" = '##form.test_location_" & #numf# & "##', ">
      <cfset session.updque &= "test_name_" & #numf# &" = '##form.test_name_" & #numf# & "##', ">
    </cfloop>
    <cfquery DATASOURCE="#ODSN#" NAME="uptest" >  
      UPDATE redbook_test SET
        <cfoutput>#PreserveSingleQuotes(updque)#</cfoutput>
        test_date_last_mod='#datecompleted#',
        test_status='C', 
        where buildno = '#session.buildno#' 
    </CFQUERY>

What do I need to do to actually get the form variable saved into the database??

How does number of input fields vary? – Dan Bracuk Jul 22 '13 at 15:23 — Dan Bracuk, Jul 22 '13 at 15:23

score 4 · Answer 1 · answered Jul 22 '13 at 15:01

4

FORM is a structure. To evaluate dynamically named fields, use associative array notation:

        <cfset value = FORM["test_location_" & numf]>

Not directly related to your question, but your current query is unsafe. CF automatically escapes single quotes inside query parameters to protect against sql injection. PreserveSingleQuotes disables that protection, putting your database at risk. To avoid that risk, always use cfqueryparam on user supplied values:

         UPDATE  Table
         SET     Column = <cfqueryparam value="#form.fieldName#" cfsqltype="...">
         WHERE   ...

Also, when you have multiple columns with the same name, ie test_location_1, test_location_2, ..., location_n it is usually a good sign you need to normalize your tables. Typically, you want to create a secondary table and store the duplicated information in rows, not columns, with a link back to the main table.

answered Jul 22 '13 at 15:01

Leigh

28,765
10
55
103

There is a button on the page that adds 5 additional columns every time clicked. There could potentially be 100 inputs on this form and I am trying to avoid running the query 100 times. That is why I'm trying to save all of the data into a string first then use that string inside the query. – Lauren Robinson Jul 22 '13 at 16:09
What happens if the user generates more form fields than you have columns? – Dan Bracuk Jul 22 '13 at 16:29
2

Forget about database calls for the moment. There are ways to minimize them - *if* needed. However, in the long run, the table structure is more important. Are you saying you are altering the table and adding **new columns** based on the number of buttons clicked? If so, that is not a good approach. First off, you will eventually hit a limit on the maximum number of columns. Second, it is difficult to optimize and awkward to query. The better approach is to store the data in rows, not columns. Then you can have as many or as few locations as needed. – Leigh Jul 22 '13 at 16:57
1

I agree with @Leigh: if possible revise the DB schema here before worrying about how to build the dynamic query. – Adam Cameron Jul 22 '13 at 19:09

Dan Bracuk · Accepted Answer · 2013-07-22T22:41:34.030

-1

This sort of syntax will get you started.

update redbook_test
set test_date_last_mod <cfqueryparam value="#test_date_last_mod#">
, test_status='C'
<cfloop list="#form.fieldnames#" index="ThisField">
<cfif left(ThisField, 5) is "test_">
, #ThisField# = <cfqueryparam value = "#form[ThisField]#">
</cfif>
</cfloop>
where buildno = <cfqueryparam value='#session.buildno#'>

It's not complete. You have to consider how to handle empty strings, different datatypes, and such.

edited Jul 22 '13 at 22:41

answered Jul 22 '13 at 16:41

Dan Bracuk

20,699
4
26
43

1

Even after fixing the syntax errors, that example is vulnerable to sql injection. Sorry, but -1. – Leigh Jul 22 '13 at 17:02
After fixing the syntax error, this did do exactly what I was asking. Thanks, Dan. But to your point, Leigh, I completely understand what you are saying and will look into normalizing the table and removing vulnerabilities. Thanks a bunch!! – Lauren Robinson Jul 22 '13 at 19:27
I see a couple of syntax errors but am curious about the vulnerability that was mentioned. – Dan Bracuk Jul 22 '13 at 22:40
(Change the if to `` will remove that risk, though I agree with going down the route of fixing the database schema.) – Peter Boughton Jul 22 '13 at 23:13
form[ThisField] is user supplied data. ThisField is generated from coldfusion and javascript code. If a user did manage to give a form field an undesired name, the left() function should throw an error due to the variable name not complying with cf's rules about such, – Dan Bracuk Jul 22 '13 at 23:44
Nope. `ThisField` is a perfectly valid variable name. CF will have no problem evaluating it. So if it contains malicious sql, it will be executed. – Leigh Jul 23 '13 at 00:38
2

(1) ThisField **is** user-supplied data. (2) You're not just dealing with users, you're dealing with attackers too; don't trust javascript content. (3) `form['; drop tables']` is entirely valid CFML. (4) Left works on a string, not a variable name. (5) When _two_ people tell you something, write some quick code to test your assumptions before replying. :) – Peter Boughton Jul 23 '13 at 12:46

Using dynamic variables in query

2 Answers2