Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

can't pass table name from variable without {} #77

Open
cpreston321 opened this issue Apr 11, 2024 · 4 comments · May be fixed by #80
Open

can't pass table name from variable without {} #77

cpreston321 opened this issue Apr 11, 2024 · 4 comments · May be fixed by #80

Comments

@cpreston321
Copy link
Member

cpreston321 commented Apr 11, 2024 •
edited
Loading

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
Copy link

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.

@amandesai01 amandesai01 linked a pull request Apr 13, 2024 that will close this issue
Open
8 tasks
@cpreston321
Copy link
Member Author

cpreston321 commented Apr 26, 2024 •
edited
Loading

@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
Copy link

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.

@amandesai01 amandesai01 mentioned this issue May 7, 2024
Closed
@rrd108
Copy link

rrd108 commented Jun 29, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

docs(guide): Add information about static parameters. amandesai01/db0
3 participants
@rrd108 @cpreston321 @amandesai01