0

I'm programming a fantasy football app in Python, and once a user has selected their initial lineup I need to write it to my SQL Server database (using pyodbc library), however this is currently throwing an error.

My entire program is about 6000 lines and counting, so far too big to print here, but here is the method containing the queries that are seemingly not working (the first index value is a player surname e.g. "Ronaldo", and the second index 'currentuser' is a numerical, auto-incremented primary key user ID value:

    def its_submit_check():

            if its_gkname != "GK" and its_d1name != "DEF" and its_d2name != "DEF" and its_d4name != "DEF" and its_d5name != "DEF" and its_m1name != "MID" and its_m2name != "MID" and its_m4name != "MID" and its_m5name != "MID" and its_f1name != "FWD" and its_f3name != "FWD" and its_s1name != "SUB" and its_s2name != "SUB" and its_s3name != "SUB" and its_s4name != "SUB" and its_s5name != "SUB":
                cursor.execute("UPDATE UserDts SET PlayerSlot1 = {0} WHERE UserID = {1}".format(its_gkname, currentuser))
                cursor.execute("UPDATE UserDts SET PlayerSlot2 = {0} WHERE UserID = {1}".format(its_d1name, currentuser))
                cursor.execute("UPDATE UserDts SET PlayerSlot3 = {0} WHERE UserID = {1}".format(its_d2name, currentuser))
                cursor.execute("UPDATE UserDts SET PlayerSlot4 = {0} WHERE UserID = {1}".format(its_d4name, currentuser))
                cursor.execute("UPDATE UserDts SET PlayerSlot5 = {0} WHERE UserID = {1}".format(its_d5name, currentuser))
                cursor.execute("UPDATE UserDts SET PlayerSlot6 = {0} WHERE UserID = {1}".format(its_m1name, currentuser))
                cursor.execute("UPDATE UserDts SET PlayerSlot7 = {0} WHERE UserID = {1}".format(its_m2name, currentuser))
                cursor.execute("UPDATE UserDts SET PlayerSlot8 = {0} WHERE UserID = {1}".format(its_m4name, currentuser))
                cursor.execute("UPDATE UserDts SET PlayerSlot9 = {0} WHERE UserID = {1}".format(its_m5name, currentuser))
                cursor.execute("UPDATE UserDts SET PlayerSlot10 = {0} WHERE UserID = {1}".format(its_f1name, currentuser))
                cursor.execute("UPDATE UserDts SET PlayerSlot11 = {0} WHERE UserID = {1}".format(its_f3name, currentuser))
                cursor.execute("UPDATE UserDts SET PlayerSlot12 = {0} WHERE UserID = {1}".format(its_s1name, currentuser))
                cursor.execute("UPDATE UserDts SET PlayerSlot13 = {0} WHERE UserID = {1}".format(its_s2name, currentuser))
                cursor.execute("UPDATE UserDts SET PlayerSlot14 = {0} WHERE UserID = {1}".format(its_s3name, currentuser))
                cursor.execute("UPDATE UserDts SET PlayerSlot15 = {0} WHERE UserID = {1}".format(its_s4name, currentuser))
                cursor.execute("UPDATE UserDts SET PlayerSlot16 = {0} WHERE UserID = {1}".format(its_s5name, currentuser))
                cursor.execute("UPDATE UserDts SET Budget = {0} WHERE UserID = {1}".format(budget, currentuser))
                
                tkMessageBox.showinfo("Initial Team Selection", "Your initial team has been selected")
                self.controller.show_frame("pitch_view_team_page")
                
            else:
                tkMessageBox.showerror("Initial Team Selection", "Please ensure you have filled every player slot with a valid selection")

And this is the exception it raises:

pyodbc.ProgrammingError: ('42000', "[42000] [Microsoft][ODBC SQL Server Driver][SQL Server]Incorrect syntax near '!'. (102) (SQLExecDirectW)") `

Normally of course you would refer to the given area where the incorrect syntax is, however there is no '!' in my queries, so I don't understand how to approach solving this.

I have many instances of CRUD methods using SQL in my program, and this is the first one to throw an error like this.

Could anybody help me please?

Dale K
  • 25,246
  • 15
  • 42
  • 71
JRandall26
  • 7
  • 5
  • What are the values of its_gkname, currentuser when the error is thrown? It looks like you are building a SQL string to execute instead of passing the values as parameters - using parameters will probably solve your problem as well as making your SQL safer. – Dale K Feb 09 '22 at 20:31
  • doesn't PlayerSlot15 = {0} need quotation marks? around the text, like PlayerSlot15 = '{0}'. Once applied, it should go to sql-server as `PlayerSlot15 ='XYZ'` – tinazmu Feb 09 '22 at 20:43
  • Note also that you can SET more than one column at a time so you could use a single UPDATE statement instead of 16 separate ones. `UPDATE table_name SET col_1 = ?, col_2 = ?, … WHERE user_id = ?` – Gord Thompson Feb 09 '22 at 20:48
  • 1
    You must have a `!` in your data. What you're doing is called SQL Injection and it's bad because you're injecting (tainted) user-supplied data right into the middle of your SQL code, so somebody could submit a currentuser value `1; drop table UserDts` and there goes all of your user data! You need to read up on Parameterized Queries, which pyodbc refers to as [Binding Parameters](https://github.com/mkleehammer/pyodbc/wiki/Binding-Parameters). – AlwaysLearning Feb 09 '22 at 21:42
  • Yes, this could be patched up in some cases by using `''`. But if you would use proper parameterization then this would never have happened in the first place. SQL injection is not just about *security*, it's also about *correctness*: it is incorrect to mix data and code. You also have a normalization issue: What happens if you decide you want another `PlayerSlot`? Add another column? Again this is a case of mixing data and definition. You should have a separate table of `PlayerSlot` normalized into rows instead of columns. – Charlieface Feb 10 '22 at 00:21
  • @Charlieface there are instances in other tables where the data is in rows i.e. users, as the number of them is dynamically changing, however in my fantasy football game there will only be 16 players in your squad, that will never change. – JRandall26 Feb 10 '22 at 09:45
  • @GordThompson Yes I am aware but for ease of coding I did them separately so it all fitted onto my screen when I was trying to look over it. – JRandall26 Feb 10 '22 at 09:46
  • @AlwaysLearning That's what I thought must be the case, but quite clearly in my queries above there is no '!', the only instance of one is in the if statement which has no relevance surely to the error? – JRandall26 Feb 10 '22 at 09:47
  • @DaleK as I mentioned in my question, the first variable is a player surname - I think it was "Fabianski" in that particular example - and the second is a numerical primary key user ID, in this case '1'. – JRandall26 Feb 10 '22 at 09:49
  • Quite clearly in your queries you are injecting the data into the query, so if the data has `!` then that is what will end up in the query. This is why you should parameterize – Charlieface Feb 10 '22 at 10:00
  • _quite clearly in my queries above there is no '!'_ While that's certainly true for your SQL statements __before__ you inject your user data we've no idea what's in your user data and how that changes the SQL statements __after__ you inject your data. I suggest you print out your SQL statements to the debug console before you pass them to `cursor.execute` - at least then you'll be able to see the exact statement causing the problem just before the exception message appears. – AlwaysLearning Feb 10 '22 at 10:04
  • @Charlieface I wouldn't know how to go about using parameters here instead though, everyone is saying to but nobody is explaining HOW I can do it. As already mentioned, I am relatively new to SQL, and this is my first time using it in Python with pyodbc. Also, the data doesn't have a '!' in it, the variables are a name and a number, so that wouldn't explain where the '!' is coming from. – JRandall26 Feb 10 '22 at 10:05
  • OK have added a couple links in the Duplicate section at the top. I didn't know you were using pyodbc so couldn't advise before. – Charlieface Feb 10 '22 at 10:09
  • @AlwaysLearning your comments have just reminded me of a problem I encountered before, different context but might be the same solution. The second variable, currentuser, is earlier defined by reading a SQL record, however I believe the first variable is the name of the widget...I in fact should be referencing 'its_gkname["text"]', and I believe that would have worked. – JRandall26 Feb 10 '22 at 10:10
  • Also, as per my previous comment: You need to read up on Parameterized Queries, which pyodbc refers to as [Binding Parameters](https://github.com/mkleehammer/pyodbc/wiki/Binding-Parameters). – AlwaysLearning Feb 10 '22 at 10:14
  • @Charlieface @AlwaysLearning having done (some quick) research on bobby-tables.com and using your links provided, would this be a better query structure: ```ps1q = "UPDATE UserDts SET PlayerSlot1 = ? WHERE UserID = ?" cursor.execute(ps1q, its_gkname["text"], currentuser)``` . Also, should I be using, in that format, ? or '%s', because the difference weren't made clear? – JRandall26 Feb 10 '22 at 10:27
  • For pyodbc use `?` – Charlieface Feb 10 '22 at 11:20

0 Answers0