can't pass table name from variable without `{}`

cpreston321 commented 2 months ago

Environment

Operating System: Darwin
Node Version: v20.10.0
Nuxt Version: 3.11.1
CLI Version: 3.11.1
Nitro Version: 2.9.5
Package Manager: bun@1.1.3
Builder: -
User Config: css, devtools, modules, fonts, shadcn, eslint, experimental, nitro
Runtime Modules: @nuxt/fonts@0.5.1, @nuxtjs/tailwindcss@6.11.4, @pinia/nuxt@0.5.1, shadcn-nuxt@0.10.2, @nuxt/eslint@0.3.0-beta.6
Build Modules: -

Reproduction

I will add one here soon.

Describe the bug

// DOESNT WORK
// routes/tables/[name].get.ts
export default defineEventHandler(async (event) => {
  try {
    const db = useDatabase()
    const name = getRouterParam(event, 'name')
    const query = getQuery(event)
    const limit = (query?.limit ?? 10) as number

    const table = await db.sql`SELECT * FROM ${name} LIMIT ${limit}`

    return table.rows ?? []
  }
  catch (error) {
    createError({
      status: 500,
      name: 'ServerError',
      cause: 'Internal Server Error',
    })
  }
})

// DOES WORK
// routes/tables/[name].get.ts
export default defineEventHandler(async (event) => {
  try {
    const db = useDatabase()
    const name = getRouterParam(event, 'name')
    const query = getQuery(event)
    const limit = (query?.limit ?? 10) as number

    const table = await db.sql`SELECT * FROM {${name}} LIMIT {${limit}}`

    return table.rows ?? []
  }
  catch (error) {
    createError({
      status: 500,
      name: 'ServerError',
      cause: 'Internal Server Error',
    })
  }
})

Additional context

It seems to work fine if I add the {} around the variable but maybe that was intended and isn't documented.

Logs

near "?": syntax error

  at Database.prepare (node_modules/better-sqlite3/lib/methods/wrappers.js:5:21)
  at Object.prepare (node_modules/db0/connectors/better-sqlite3.mjs:24:29)
  at Object.sql (node_modules/db0/dist/index.mjs:34:38)
  at Object.handler (server/api/tables/[name].get.ts:9:1)
  at node_modules/h3/dist/index.mjs:1890:43
  at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
  at async node_modules/h3/dist/index.mjs:1962:19
  at async Object.callAsync (node_modules/unctx/dist/index.mjs:72:16)
  at async Server.toNodeHandle (node_modules/h3/dist/index.mjs:2249:7)

amandesai01 commented 2 months ago

May I know what is the use-case for this? Because prepared statements are not supported for table names.

Now describing what happened in your case:

Case 1 which doesn't work: DB0 thought that the name is meant to be used as prepared statement and following query was generated: SELECT * FROM ? LIMIT ?. DB connector rejected that since table name is expected there.
Case 2 which does work (wrapped by braces): DB0 has took it as a static parameter (parameter which need not be passed as prepared statement). This is to allow developer to use string literals without them being passed on as connection parameters. Yes this should be documented, I will raise the PR.

Important Note:

If this was for demonstration, it's okay. But never send static parameters from an untrusted source such as request. STATIC PARAMETERS ARE NOT SANITISED.

cpreston321 commented 2 months ago

@amandesai01 I was creating a endpoint to dynamically fetch the table name e.g. GET /tables/users then would return this to the frontend to render the table. Maybe there is a better way to go about it, but I don't want to create multiple endpoints to fetch all the tables 😅

This application I was building was to view the data on the application side and the user will select what table they want to view. Almost like DataGrip, Table Plus etc..

amandesai01 commented 2 months ago

You must use some enums / strict validation. You may create a set of valid values and make sure value exists in set before sending it to query or whatever.

rrd108 commented 6 days ago

I run into this today. Thanks for the {} solution.

unjs / db0