mariadb-corporation / mariadb-connector-nodejs

MariaDB Connector/Node.js is used to connect applications developed on Node.js to MariaDB and MySQL databases. MariaDB Connector/Node.js is LGPL licensed.
GNU Lesser General Public License v2.1
363 stars 93 forks source link

UPDATE query not working and providing insufficient error #241

Closed Myrmod closed 1 year ago

Myrmod commented 1 year ago

Hello, I am creating an UPDATE query as follows:

    const queryResult = await connection.query(
        {
            sql: `UPDATE users SET ? WHERE id = ${user.id}`,
            autoJsonMap: true,
        },
        {
            name: twoWayEncrypt(user.name),
            email: twoWayEncrypt(user.email),
            password: user.password,
            functionality: user.functionality ? JSON.stringify(user.functionality) : undefined,
            accessRights: user.accessRights ? JSON.stringify(user.accessRights) : undefined,
        },
    )

which results in the following error:

SqlError: 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 ''{\"name\":\"762f1fb0cc57076b0a49e8f33d86a9bf3b3cb27e9a0095b4af2a0fb992fd49f4...' at line 1
sql: UPDATE users SET ? WHERE id = 2 - parameters:[{"name":"762f1fb0cc57076b0a49e8f33d86a9bf3b3cb27e9a0095b4af2a0fb992fd49f47bc667e7c033da4a7c0fe3d9af48a147","email":"a1771b3849f40c7ff2d13f8451ba1f8680e01c1e2ec68bbe8d243e1ddb565036bf900d8e3f92824d7ca8975b547f13...]

I don't see the problem and I couldn't find any documentation on this issue.

rusher commented 1 year ago

I would recommand to post that to stackoverflow. This is a sequelize configuration issue and i'm not a specialist of sequelize. All i can say the that the problem here is that the query connector receive is UPDATE users SET ? WHERE id = 2 with some parameters. I imagine that what is expected is passing query UPDATE users SET name=?,email=?,password=?,functionality=?,accessRights=? WHERE id = 2 with some parameters.

Sequelize build query depending on parameters. So depending on your sequelize version, check documentation like here it might be something like :

await sequelize.query(
  'UPDATE users SET name=?,email=?,password=?,functionality=?,accessRights=?  WHERE id = ?',
  {
   replacement: [
            twoWayEncrypt(user.name),
            twoWayEncrypt(user.email),
            user.password,
            user.functionality ? JSON.stringify(user.functionality) : undefined,
            user.accessRights ? JSON.stringify(user.accessRights) : undefined,
                        user.id,
        ]
  },
  type: QueryTypes.UPDATE,
);
Myrmod commented 1 year ago

thank you, I'll give it a try

rusher commented 1 year ago

if i'm wrong and that not sequelize at all ('autoJsonMap' made me thing of that) then that's a little bit different. either do :

    const queryResult = await connection.query(
        'UPDATE users SET name=?,email=?,password=?,functionality=?,accessRights=?  WHERE id = ?',
        [
            twoWayEncrypt(user.name),
            twoWayEncrypt(user.email),
            user.password,
            user.functionality ? JSON.stringify(user.functionality) : undefined,
            user.accessRights ? JSON.stringify(user.accessRights) : undefined,
            user.id,
        },
    )

or passing named parameters:

    const queryResult = await connection.query(
        { namedPlaceholders: true, sql:'UPDATE users SET name=:name,email=:email,password=:password,functionality=:functionality,accessRights=:accessRights WHERE id = :id'},
        {
            name: twoWayEncrypt(user.name),
            email: twoWayEncrypt(user.email),
            password: user.password,
            functionality: user.functionality ? JSON.stringify(user.functionality) : undefined,
            accessRights: user.accessRights ? JSON.stringify(user.accessRights) : undefined,
            id: user.id
        },
    )
Myrmod commented 1 year ago

I tried both of these with the same error. So what I ended up doing is the following:

let setValues = []
setValues.push(user.name ? `name = "${twoWayEncrypt(user.name)}"` : undefined)
...
setValues.push(
  user.functionality ? `functionality = '${JSON.stringify(user.functionality)}'` : undefined,
)
setValues = setValues.filter(v => v)
await connection.query({
  sql: `UPDATE users SET ` + setValues.join(', ') + ` WHERE id = ${user.id}`,
  autoJsonMap: true,
})

This works and is not as badly readable as I imagined.

rusher commented 1 year ago

If ^^ works for you, i would still recommand passing parameters in order to prevent SQL injection. exploits_of_a_mom

Myrmod commented 1 year ago

Hahaha, nice comic strip!

Yea SQL injection might be a problem. I'm already checking it at multiple places, but more doesn't hurt. Thank you!