1

What I'm aiming to do is create a few functions which will assist me in the creation of a multitude of MySqlCommand. But I'm going wrong (information provided below).

Background

I have achieved this by defining constants in a class, such as:

Public Const Create_Table = "CREATE TABLE IF NOT EXISTS `@arg0` (@arg1)"

Strings such as this form the basis for my parameterised query. Now, these arguments aren't single values, they are actually concatenated strings. So I know I'm edging towards dangerous waters here, so bear with me here.

I use the following function to concatenate arguments:

Public Shared Function ListToQuery(values As List(Of String),
                                   Optional ByVal separator As String = ", ") As String
    Dim queryBuilder As New StringBuilder

    For Each value As String In values
        queryBuilder.Append((value) & separator)
    Next
    Dim query As String = queryBuilder.ToString
    Return query.Remove(query.Length - 2)
End Function

This works as per normal; when I call it with a List:

Dim myValues As New List(Of String)
myValues.AddRange({"Int Primary", "Text Name"})
MsgBox(ListToQuery(myValues))

It returns "Int Primary, Text Name" - perfectly normal. However, this is where things start to go wrong.

My next part is to create the MySqlCommand based off several arguments, an Array of String. This is achieved by calling the following function:

Public Shared Function BuildCommand(args() As String, Command As String, connection As MySqlConnection) As MySqlCommand

    Dim cmd As New MySqlCommand(Command, connection)
    For i As Integer = 0 To args.Length - 1
        'cmd.CommandText = cmd.CommandText.Replace("@arg" & i, args(i))
        cmd.Parameters.AddWithValue("@arg" & i, MySqlHelper.EscapeString(args(i)))
    Next
    Return cmd

End Function

Issue

I call this function by doing this:

Dim myCommand as MySqlCommand = BuildCommand({"MyTableName", ListToQuery(myValues)}, Create_Table, myConnection)
Dim dr = myCommand.ExecuteReader()

What seems to happen, here is that an error occurs, and I picked up something which seems to occur within the parameterised query:

You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ''Int Primary, Text Name')' at line 1

The argument seems to have been encapsulated within ', if I'm interpreting this error correctly. Now note the comment in my code for BuildCommand(). If I were to use pure concatenation, that query would run just fine.

Edit: The resulting query should have been:

CREATE TABLE IF NOT EXISTS `MyTableName` (Int Primary, Text Name)

Question

I have a couple, in fact. Firstly, I would like to ask if this is a safe way (i.e. even with concatenating my arguments and using parameters in the query) to run a command, and secondly, how can I avoid getting this error?

I'm not quite sure if I can parse each argument individually without causing the query to become 'unsafe' or without modifying the general MySQL statement.

I've been discussing this with another user and he managed to find this question: Parameterized dynamic sql query. This is asking a similar question. However, it is not the same issue I'm having.

Thanks in advance.

Visual Vincent
  • 18,045
  • 5
  • 28
  • 75
Ally
  • 467
  • 5
  • 24
  • First: Linking pictures is not the best idea... Second: Please don't use .AddWithValue. Third: Can you give an example which query should be generated, because I did not really catch what you want to do. – muffi Sep 06 '17 at 12:46
  • @muffi, what would you suggest I use as an alternative to `.AddWithValue()` - and why shouldn't I use it? Also, the query generated, in this example, should have been `CREATE TABLE IF NOT EXISTS ``MyTableName`` (Int Primary, Text Name)` (don't mind the double ` - syntax formatting in comments is annoying) – Ally Sep 06 '17 at 12:47
  • .AddWithValue is not a good idea, because there are some situations where the db-type is wrong. The other thing: I don't know a way to make what you want, only a workaround. See it in a few minutes in the answer. – muffi Sep 06 '17 at 13:04
  • I don't think you can use parameters for that. It would be like trying to do "INSERT INTO @tableNameAsVariable". – the_lotus Sep 06 '17 at 13:26
  • @the_lotus what do you mean...? I'm pretty sure you can :/ – Ally Sep 06 '17 at 13:38

2 Answers2

0

This function generates a CREATE TABLE command. I got the same problem several times, but until now I did not find a suitable solution. Instead I create my query my own:

    Public Shared Function BuildCommand(args() As String, connection As MySqlConnection) As MySqlCommand
    Dim sb As New StringBuilder
    Dim cmd As New MySqlCommand
    cmd.Connection = connection
    sb.Append("CREATE TABLE IF NOT EXISTS`")
    sb.Append(args(0))
    sb.Append("`(")
    For i As Integer = 1 To args.Length - 1
        sb.Append("`")
        sb.Append(args(i))
        sb.Append("`,")
    Next
    sb.Remove(sb.Length - 1, 1)
    sb.Append(")")
    cmd.CommandText = sb.ToString
    Return cmd
End Function
muffi
  • 366
  • 6
  • 18
  • That completely destroyed the purpose of my function. It was to provide a versatile base function which can be utilised by commands with *different syntaxes*. Furthermore, can I ask, why are you calling `sb.Append` so many times? – Ally Sep 06 '17 at 13:14
  • Alex, the connection is in the arguments of the function (you coded it the same way ;-) ). Because i killed the `Command` parameter, I needed to tell the `cmd` which connection it should use. I don't create a new connection! The sb is a StringBuilder. It is much more faster than concatenating like `str = str & "newstring"`. – muffi Sep 06 '17 at 13:17
  • By 'why are you calling sb.Append so many times?' I mean, would there be a better way to do it @muffi? Also, I realised, my eyes played a trick (connection). – Ally Sep 06 '17 at 13:17
  • You know the class StringBuilder? The name says everything :-) With the method .Append, I append a string in the StringBuilder... ;-) What should I say more, it is not more ;-) – muffi Sep 06 '17 at 13:25
  • I know what it does. Either way, this method isn't going to work with other queries. For that reason I'm not accepting this answer, sorry. – Ally Sep 06 '17 at 13:38
0

This code is not the same as the one you showed me before, but now that I see what changes you've made and in what context you use this things just got much easier.

When adding SQL parameters you do not need to escape their value since that is done automatically. This is probably why you experience it getting escaped twice.

Change:

cmd.Parameters.AddWithValue("@arg" & i, MySqlHelper.EscapeString(args(i)))

to:

cmd.Parameters.AddWithValue("@arg" & i, args(i))

...and it should work (and be safe)!

Visual Vincent
  • 18,045
  • 5
  • 28
  • 75
  • Unfortunately I originally tried that, it still encapsulated the parameter in `''`, strangely enough. – Ally Sep 06 '17 at 23:06
  • @AlexM. : It should encapsulate the parameter in _**one**_ set of `'`, otherwise the value can't be used properly (for instance it cannot contain spaces). When performing regular, unsafe concatenation that's what you do as well: `"'" & TextBox1.Text & "'"`. – Visual Vincent Sep 07 '17 at 02:49
  • @AlexM. : What if you try this instead: `cmd.Parameters.Add("@arg" & i, SqlDbType.VarChar).Value = args(i)`? – Visual Vincent Sep 07 '17 at 02:56
  • `[ERROR] You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ''ID Int, Blah1 BigInt, PRIMARY KEY (ID)')' at line 1` - same issue. – Ally Sep 07 '17 at 11:09
  • Hmm, perhaps you cannot use strings with spaces? What happens if you try something simple like: `args(0) = "Hello"`? Though it seems the problem is that it adds an extra bracket at the end for some reason: `')'`. – Visual Vincent Sep 07 '17 at 14:45
  • Nope it's not the extra bracket. The exception only shows the part of the query that's causing an issue- this means that it's the `'` causing the issue. I think I figured it out; but it's not something I can change. – Ally Sep 07 '17 at 23:48
  • 1
    @AlexM. : Long time no see :). That was a rather late accept, I thought this didn't work for you... What changed? – Visual Vincent Feb 23 '18 at 07:44
  • I was misusing it... it's not meant for names, only values, which would thus make this answer valid and more simple. Thanks. – Ally Feb 23 '18 at 11:11