Error running system.db.runPrepQuery with spaces in values

Hello,

I am encountering an error message when running system.db.runPrepQuery with spaces in the values which will be written to SQL cells. Is there a way to run this and still use spaces?

Here is the button script, which works fine as long as the strings don’t contain spaces:

def convertTuple(tup):  #converts a tuple into a string
    str =  ','.join(tup) 
    return str
    
def convertList(list):  	#converts a list into a tuple
    return tuple(list) 


#this checks to ensure that the key column value is not a duplicate
Key = event.source.parent.getComponent('Field1').text

query = "SELECT Measure_Index FROM Measurement_Data WHERE Measure_Index = ?"
data = system.db.runPrepQuery(query, [Key])

L1 = event.source.parent.getComponent('Label1').text
L2 = event.source.parent.getComponent('Label2').text
L3 = event.source.parent.getComponent('Label3').text
L4 = event.source.parent.getComponent('Label4').text
L5 = event.source.parent.getComponent('Dropdown5').selectedStringValue
L6 = event.source.parent.getComponent('Dropdown6').selectedStringValue
L7 = event.source.parent.getComponent('Label7').text
L8 = event.source.parent.getComponent('Label8').text
L9 = event.source.parent.getComponent('Label9').text
L10 = event.source.parent.getComponent('Label10').text
L11 = event.source.parent.getComponent('Label11').text
L12 = event.source.parent.getComponent('Label12').text

F1 = "'" + event.source.parent.getComponent('Field1').text + "'"
F2 = "'" + event.source.parent.getComponent('Field2').text  + "'"
F3 = "'" + event.source.parent.getComponent('Field3').text  + "'"
F4 = "'" + event.source.parent.getComponent('Field4').text + "'"
F5 = "'" + event.source.parent.getComponent('Dropdown5').selectedStringValue + "'"
F6 = "'" + event.source.parent.getComponent('Dropdown6').selectedStringValue + "'"
F7 = "'" + event.source.parent.getComponent('Field7').text + "'"
F8 = "'" + event.source.parent.getComponent('Field8').text + "'"
F9 = "'" + event.source.parent.getComponent('Field9').text + "'"
F10 = "'" + event.source.parent.getComponent('Field10').text + "'"
F11 = "'" + event.source.parent.getComponent('Field11').text + "'"
F12 = "'" + event.source.parent.getComponent('Field12').text + "'"

Ls = (L1,L2,L3,L4,L5,L6,L7,L8,L9,L10,L11,L12)
Fs = [F1, F2, F3, F4, F5, F6, F7, F8, F9, F10, F11, F12]


if data.rowCount == 0:
	#if key value is not a duplicate, code continues here
	if (F3 == '') or (F4 == '') or (F5 == '') or (F6 == '') or (F7 == '') or (F7 == '') or (F9 == '') or (F12 == ''): #Ensure mandatory fields are filled
		errorMessage = "Please fill all fields marked with *"
		system.gui.errorBox(errorMessage)
		
	else:
		if system.gui.confirm("Are you sure you want to create this new entry? You cannot undo this action."):
			Headers = convertTuple(Ls) #converts SqlLabels tuple into string for transaction
			SqlValuesList = convertList(Fs) #converts from tuple to list for SQL transaction
	
			system.db.runPrepUpdate("INSERT INTO Measurement_Data (%s) VALUES (%s)" % ((Headers),','.join(SqlValuesList)))
		
			system.gui.closeParentWindow(event)
else:
	errorMessage = "The Measure_Index number already exists, this could indicate someone is entering data at the same time as you. Please cancel out and try again."
	system.gui.errorBox(errorMessage)


Here is the error from running this script on a button when one of the values contains a space:

Traceback (most recent call last):
File “event:actionPerformed”, line 56, in
java.lang.Exception: java.lang.Exception: Error executing system.db.runPrepUpdate(INSERT INTO Measurement_Data (Measure_Index,LotNumber,SpoolID,LineNumber,Adhesion,Peel Strength,MeasurementNumber,MeasurementValue,Unit,Result,Comment,Initials) VALUES (‘7’,‘1’,‘1’,‘254’,‘Adhesion’,‘Peel Strength’,‘1’,‘1’,‘Test’,‘Test’,‘TestEntry’,‘ABC’), [null], , , false, false)

caused by Exception: Error executing system.db.runPrepUpdate(INSERT INTO Measurement_Data (Measure_Index,LotNumber,SpoolID,LineNumber,Adhesion,Peel Strength,MeasurementNumber,MeasurementValue,Unit,Result,Comment,Initials) VALUES ('7','1','1','254','Adhesion','Peel Strength','1','1','Test','Test','TestEntry','ABC'), [null], , , false, false)
caused by GatewayException: SQL error for "INSERT INTO Measurement_Data (Measure_Index,LotNumber,SpoolID,LineNumber,Adhesion,Peel Strength,MeasurementNumber,MeasurementValue,Unit,Result,Comment,Initials) VALUES ('7','1','1','254','Adhesion','Peel Strength','1','1','Test','Test','TestEntry','ABC')": Incorrect syntax near 'Strength'.
caused by SQLServerException: Incorrect syntax near 'Strength'.

Ignition v8.1.1 (b2020120808)
Java: Azul Systems, Inc. 11.0.7

Thanks for your help.

Are you sure the problem is with your value that contains a space and not your header/column that contains a space (“Peel Strength”)?

Oh my, I must need another coffee. You’re right Kevin, I added the dropdown selections instead of the text fields and changed them not only in the value section, but also in the header section. Thanks for your help!

Just fyi, since you’re using runPrepUpdate, you don’t need to manually add quotation marks to your values. It knows how to handle string objects, date objects, etc.

@bkarabinchak.psi - I thought it wouldn’t need them, but I tried it without the apostrophes and it errors out without them. Maybe it has to do with how I’m combining the values, I’m not sure.

It's because of the way you are providing the values to the function.

Using system.db.runPrepUpdate() in this manner completely circumvents the entire purpose of using the function to begin with. This script is very vulnerable to a SQL Injection Attack.

What you should be doing instead is providing a list of values in the args parameter of the function, I'm kind of surprised that the function executes with a NoneType value for the args parameter anyway considering the purpose of the function.

You should be building a string with a '?' provided for each of the values that you have. Something like:

system.db.runPrepUpdate('INSERT INTO Measurement_Data (%s) VALUES (%s)' % ( Headers,','.join(['?'] * len(SqlValuesList))),SqlValuesList)

Doing it that way will allow the function to manage the object data types for you.

3 Likes

@lrose is right use ? instead of providing the values via string interpolation and it will make your life a lot easier.

One other thing - I see you you are storing integers as strings as in VALUES ('7','1','1','254'. Unless it’s absolutely necessary, you might want to reconsider this. What happens if you need to get all rows where the value is greater than 254? You can’t do greater than/less than operations on strings easily. Good database design is essential imho and often overlooked, leading to avoidable headaches.

3 Likes

Thank you all for your help and tips,

@lrose , I’m doing it this way because it is what you showed me in a previous post: http://forum.inductiveautomation.com/t/use-system-db-runupdatequery-with-variable-value-counts/42658/3?u=rusty
I don’t know how to use that command otherwise with a variable number of arguments.

@bkarabinchak.psi The only reason I’m converting it to a string is because I can’t get the values to go in the command without erroring out unles each value is enclosed in apostrophes. The columns in the datatable are set as integers, so the values end up being integers there.

I remember the post, the difference is, in that post you were using system.db.runUpdateQuery() which would require you do it that way. system.db.runPrepUpdate() is specifically meant to handle queries with dynamic arguments.

From the manual:

This method avoids the problematic technique of concatenating user input inside of a query, which can lead to syntax errors, or worse, a nasty security problem called a SQL injection attack .

Build the list of columns and values the way you have, but instead of inserting the values directly into the query string, instead build a string of placeholders (e.g. '?,?,?,?') and then supply the values list as the args parameter in the function.

#['?'] * len(SqlValuesList) returns a list of strings the length of the SqlValuesList
# where each element is '?', so if SqlValuesList has 4 values then your returned list would look like 
#['?','?','?','?']. Then the string.join function can be used to return a string built from that list.
sValues = ','.join(['?'] * len(SqlValuesList))

#once you have that then you can use that to build your query string
query = 'Insert INTO Measurement_Data (%s) Values (%s)' % (Headers,sValues)

#Now you can call the function
system.db.runPrepUpdate(query,SqlValuesList)
2 Likes

@lrose Thanks for clarifying, I’ll try it out!

@lrose Thank you for helping me to understand this. I have implemented the changes you suggested and it works. Is this method safer against SQL injections?

#This script is used to gather the values entered into the manual 
#data entry form and to send the values into a SQL table row


###########################################################
#Variables
##########################################################

L1 = event.source.parent.getComponent('Label1').text
L2 = event.source.parent.getComponent('Label2').text
L3 = event.source.parent.getComponent('Label3').text
L4 = event.source.parent.getComponent('Label4').text
L5 = event.source.parent.getComponent('Label5').text
L6 = event.source.parent.getComponent('Label6').text
L7 = event.source.parent.getComponent('Label7').text
L8 = event.source.parent.getComponent('Label8').text
L9 = event.source.parent.getComponent('Label9').text
L10 = event.source.parent.getComponent('Label10').text
L11 = event.source.parent.getComponent('Label11').text
L12 = event.source.parent.getComponent('Label12').text

F1 = event.source.parent.getComponent('Field1').text
F2 = event.source.parent.getComponent('Field2').text
F3 = event.source.parent.getComponent('Field3').text
F4 = event.source.parent.getComponent('Field4').text
F5 = event.source.parent.getComponent('Dropdown5').selectedStringValue
F6 = event.source.parent.getComponent('Dropdown6').selectedStringValue
F7 = event.source.parent.getComponent('Field7').text
F8 = event.source.parent.getComponent('Field8').text
F9 = event.source.parent.getComponent('Field9').text
F10= event.source.parent.getComponent('Field10').text
F11= event.source.parent.getComponent('Field11').text
F12= event.source.parent.getComponent('Field12').text

Ls = (L1,L2,L3,L4,L5,L6,L7,L8,L9,L10,L11,L12)
Fs = [F1, F2, F3, F4, F5, F6, F7, F8, F9, F10, F11, F12]

###########################################################
#definitions
###########################################################

def convertTuple(tup):  #converts a tuple into a string
    str =  ','.join(tup) 
    return str
    
def convertList(list):  	#converts a list into a tuple
    return tuple(list) 

###########################################################
#execution
###########################################################

#this checks to ensure that the key column value is not a duplicate
Key = event.source.parent.getComponent('Field1').text
query = "SELECT Measure_Index FROM Measurement_Data WHERE Measure_Index = ?"
data = system.db.runPrepQuery(query, [Key])

if data.rowCount == 0:
	#if key value is not a duplicate, code continues here
	if (F3 == '') or (F4 == '') or (F5 == '') or (F6 == '') or (F7 == '') or (F7 == '') or (F9 == '') or (F12 == ''): #Ensure mandatory fields are filled
		errorMessage = "Please fill all fields marked with *"
		system.gui.errorBox(errorMessage)
		
	else: #confirm before submitting values
		if system.gui.confirm("Are you sure you want to create this new entry? You cannot undo this action."):
			#converts from tuple to list for SQL transaction
			SqlValuesList = convertList(Fs)
			#['?'] * len(SqlValuesList) returns a list of strings the length of the SqlValuesList
			# where each element is '?', so if SqlValuesList has 4 values then your returned list would look like 
			#['?','?','?','?']. Then the string.join function can be used to return a string built from that list.
			sValues = ','.join(['?'] * len(SqlValuesList))
			#converts SqlLabels tuple into string for transaction
			Headers = convertTuple(Ls)
			#once you have that then you can use that to build your query string
			query = 'Insert INTO Measurement_Data (%s) Values (%s)' % (Headers,sValues)
			
			#Now you can call the function
			system.db.runPrepUpdate(query,SqlValuesList)
				
			#Close entry form pop-up window		
			system.gui.closeParentWindow(event)
			
else: #Error message, key value is not unique
	errorMessage = "The Measure_Index number already exists, this could indicate someone is entering data at the same time as you. Please cancel out and try again. If the problem persists, contact your system administrator"
	system.gui.errorBox(errorMessage)



Yes, this will be safer against SQL Injections

1 Like