Skip to content

Using pg-native to INSERT gzip binary buffer will result in wrong value #113

@dr-js

Description

@dr-js
  • expect sends: 1f8b080000000000000373700852d0e2acaee57270f08550219cd5b5001d44c15f16000000 (hex)
  • actual saved: 1fefbfbd08 (hex)

the wrong value 1fefbfbd08 is the first 5-bytes of source value, after JS UTF8 encode & decode:

> Buffer.from(Buffer.from('1f8b080000000000000373700852d0e2acaee57270f08550219cd5b5001d44c15f16000000','hex').toString())
<Buffer 1f ef bf bd 08 00 00 00 00 00 00 03 73 70 08 52 ef bf bd e2 ac ae ef bf bd 72 70 ef bf bd ef bf bd 50 21 ef bf bd d5 b5 00 1d 44 ef bf bd 5f 16 00 00 ... 1 more byte>

some debugging

add log to sendQueryParams in node-libpq to show value passed to C++: https://github.com/brianc/node-libpq/blob/v1.8.13/index.js#L145-L155

console.log('[sendQueryParams]', commandText, parameters, new Error().stack)

# [sendQueryParams] INSERT INTO "fltre" ("id", "t10s", "size", "tre") VALUES ($1, $2, $3, $4); [
#   '1',
#   '1',
#   '22',
#   <Buffer 1f 8b 08 00 00 00 00 00 00 03 73 70 08 52 d0 e2 ac ae e5 72 70 f0 85 50 21 9c d5 b5 00 1d 44 c1 5f 16 00 00 00>
# ] Error:
#     at PQ.sendQueryParams (.../node_modules/libpq/index.js:163:61)
#     at fn (.../node_modules/pg-native/index.js:62:22)
#     at module.exports._dispatchQuery (.../node_modules/pg-native/index.js:290:16)
#     at module.exports.query (.../node_modules/pg-native/index.js:70:8)
#     at module.exports.submit (.../node_modules/pg/lib/native/query.js:159:19)
#     at module.exports._pulseQueryQueue (.../node_modules/pg/lib/native/client.js:283:9)
#     at module.exports.query (.../node_modules/pg/lib/native/client.js:234:8)
#     at PendingItem.callback (.../node_modules/pg-pool/index.js:429:16)
#     at BoundPool._acquireClient (.../node_modules/pg-pool/index.js:321:21)
#     at BoundPool._pulseQueue (.../node_modules/pg-pool/index.js:156:19)
#     at .../node_modules/pg-pool/index.js:195:37
#     at processTicksAndRejections (node:internal/process/task_queues:77:11)

add log option to database to check actual reveived value:

docker run --name dev-pg --rm -p 5432 postgres:9.2 -c log_statement=all

# LOG:  execute <unnamed>: INSERT INTO "fltre" ("id", "t10s", "size", "tre") VALUES ($1, $2, $3, $4);
# DETAIL:  parameters: $1 = '1', $2 = '1', $3 = '22', $4 = '\x1fefbfbd08'
Related code digging for the stacktrace above

from cpp code for char** Connection::NewCStringArray(v8::Local<v8::Array> jsParams): https://github.com/brianc/node-libpq/blob/v1.8.13/src/connection.cc#L786-L789

    //expect every other value to be a string...
    //make sure aggresive type checking is done
    //on the JavaScript side before calling
    parameters[i] = NewCString(val);

expect all values to be string

from js code for PQ.prototype.sendQueryParams: https://github.com/brianc/node-libpq/blob/v1.8.13/index.js#L145-L155

PQ.prototype.sendQueryParams = function (commandText, parameters) {
  if (!commandText) {
    commandText = '';
  }
  if (!parameters) {
    parameters = [];
  }

  assert(Array.isArray(parameters), 'Parameters must be an array');
  return this.$sendQueryParams(commandText, parameters);
};

no check on each parameters type

from js code for pg-native:
https://github.com/brianc/node-postgres/blob/8f8e7315e8f7c1bb01e98fdb41c8c92585510782/packages/pg-native/index.js#L53-L74

Client.prototype.query = function (text, values, cb) {
  let queryFn

  if (typeof values === 'function') {
    cb = values
  }

  if (Array.isArray(values)) {
    queryFn = () => {
      return this.pq.sendQueryParams(text, values)
    }
  } else {
    queryFn = () => {
      return this.pq.sendQuery(text)
    }
  }

  this._dispatchQuery(this.pq, queryFn, (err) => {
    if (err) return cb(err)
    this._awaitResult(cb)
  })
}

no check on each parameters type

from js code for pg:
https://github.com/brianc/node-postgres/blob/8f8e7315e8f7c1bb01e98fdb41c8c92585510782/packages/pg/lib/native/query.js#L153-L159

  } else if (this.values) {
    if (!Array.isArray(this.values)) {
      const err = new Error('Query values must be an array')
      return after(err)
    }
    const vals = this.values.map(utils.prepareValue)
    client.native.query(this.text, vals, after)

the values are mapped with utils.prepareValue:
https://github.com/brianc/node-postgres/blob/8f8e7315e8f7c1bb01e98fdb41c8c92585510782/packages/pg/lib/utils.js#L50-L58

const prepareValue = function (val, seen) {
  // null and undefined are both null for postgres
  if (val == null) {
    return null
  }
  if (typeof val === 'object') {
    if (val instanceof Buffer) {
      return val
    }

which do not convert Buffer to encoded data string

Basically, node-libpq expect all values to be string, but upper code will send Buffer directly through.

Normal text buffer won't show the error, but binary buffer with invalid UTF8 pair will be passed as different value.

A quick fix in JS side can be done by pre-encoding the buffer before send:

buf = `\\x${buf.toString('hex')}`

But for better performance, I think the buffer encoding should be handled in cpp code.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions