brianc / node-pg-pool

A connection pool for node-postgres
MIT License
180 stars 64 forks source link

Using quotes in query ignores parameters inside the quotes #80

Closed modulitos closed 6 years ago

modulitos commented 6 years ago

For example, I was running the following query:

const query =
  `
  SELECT statefp, name, stusps
  FROM my_table as tb
  WHERE st_contains(tb.geom, st_geomfromtext('POINT($1 $2)', 4269))
`
const states = await client.query(
 query,
 [ longitude, latitude ]
)

which gave me the following error:

{ error: bind message supplies 2 parameters, but prepared statement "" requires 0
    at Connection.parseE (/home/lucas/projects/docker-projects/political-areas/node_modules/pg/lib/connection.js:546:11)
    at Connection.parseMessage (/home/lucas/projects/docker-projects/political-areas/node_modules/pg/lib/connection.js:371:19)
    at Socket.<anonymous> (/home/lucas/projects/docker-projects/political-areas/node_modules/pg/lib/connection.js:114:22)
    at emitOne (events.js:115:13)
    at Socket.emit (events.js:210:7)
    at addChunk (_stream_readable.js:266:12)
    at readableAddChunk (_stream_readable.js:253:11)
    at Socket.Readable.push (_stream_readable.js:211:10)
    at TCP.onread (net.js:585:20)
  name: 'error',
  length: 130,
  severity: 'ERROR',
  code: '08P01',
  detail: undefined,
  hint: undefined,
  position: undefined,
  internalPosition: undefined,
  internalQuery: undefined,
  where: undefined,
  schema: undefined,
  table: undefined,
  column: undefined,
  dataType: undefined,
  constraint: undefined,
  file: 'postgres.c',
  line: '1556',
  routine: 'exec_bind_message' }

It's as if my query wasn't accepting any parameters, but as we can clearly see above, it takes the $1 and $2 parameters. After about an hour of trying to figure out what went wrong, I realized that my parameters are inside of a quoted string (ie 'POINT($1 $2)'). Perhaps this is a bug, or a feature, where params inside of a quoted string are not parsed by pg-pool's client?

I think this strange behavior should at least be documented. If so, I'm happy to help with that.

In the end, the following refactor fixed it:

const query =
  `
  SELECT statefp, name, stusps
  FROM my_table as tb
  WHERE st_contains(tb.geom, st_geomfromtext('POINT(${longitude} ${latitude})', 4269))
`
const states = await client.query(
 query
)
charmander commented 6 years ago

Parameters are a PostgreSQL concept, not just a pg invention. They’re expressions and don’t work by simple string replacement. This is consistent with how you would select a string:

pool.query(`SELECT $1`, ['example'])

and not

pool.query(`SELECT '$1'`, ['example'])

One safe correction to your example (${longitude} ${latitude} is unsafe by default and could lead to SQL injection):

const query =
  `
  SELECT statefp, name, stusps
  FROM my_table as tb
  WHERE st_contains(tb.geom, st_geomfromtext('POINT(' || $1 || ' ' || $2 || ')', 4269))
  `
const states = await client.query(
 query,
 [ longitude, latitude ]
)
sehrope commented 6 years ago

Rather than serializing and bind the geo types as text, you can directly create a point via: POINT(x,y)

modulitos commented 6 years ago

@charmander @sehrope Thanks for the explanation - this makes a lot more sense now.

Closing this issue :-)