Malformed array literal: Knex 2.4.0 regression

[orcwarrior]

Environment

Knex version: 2.4.0
Database + version: postgre 14
OS: windows & unix

Bug

1. Explain what kind of behaviour you are getting and how you think it should do

2. Error message

insert into "MetaRequiredFields" ("requiredFields", "tableName") values ($1, $2), ($3, $4) - malformed array literal: "["lineItemText","cost","taxPercentage","priceMarkup","unit"]"
    at Parser.parseErrorMessage (C:\Projects\trackbuild\server\node_modules\pg-protocol\src\parser.ts:369:69)
    at Parser.handlePacket (C:\Projects\trackbuild\server\node_modules\pg-protocol\src\parser.ts:188:21)
    at Parser.parse (C:\Projects\trackbuild\server\node_modules\pg-protocol\src\parser.ts:103:30)
    at Socket.<anonymous> (C:\Projects\trackbuild\server\node_modules\pg-protocol\src\index.ts:7:48)
    at Socket.emit (node:events:513:28)
    at Socket.emit (node:domain:489:12)
    at addChunk (node:internal/streams/readable:324:12)
    at readableAddChunk (node:internal/streams/readable:297:9)
    at Socket.Readable.push (node:internal/streams/readable:234:10)
    at TCP.onStreamRead (node:internal/stream_base_commons:190:23) {
  length: 240,
  severity: 'ERROR',
  code: '22P02',
  detail: '"[" must introduce explicitly-specified array dimensions.',
  hint: undefined,
  position: undefined,
  internalPosition: undefined,
  internalQuery: undefined,
  where: "unnamed portal parameter $1 = '...'",
  schema: undefined,
  table: undefined,
  column: undefined,
  dataType: undefined,
  constraint: undefined,
  file: 'arrayfuncs.c',
  line: '269',
  routine: 'array_in'
}

3. No testcode included

Hello! due to investion in my own Project I was able to pinpoint issue with Knex in just released 2.4.0 version.
One of my table rows uses a text array (_text) as type. Until 2.4.0 I'd inserted js-array as values w/o any problem.
However new version introduces the error posted above. I;' #5321 PR just introduced to be a cause.

[orcwarrior]

Just Confirmed that 2.3.0 downgrade fixes the issue

[nebulae]

I'm also experiencing this breaking issue, confirmed downgrading fixed.

[florianakos]

After trying to upgrade to 2.4.0 to fix the CVE, our tests started failing, exmple:

     error: malformed array literal: "["10.150.145.116","10.5.43.154","218.167.177.211"]"
      at Parser.parseErrorMessage (node_modules/pg-protocol/dist/parser.js:287:98)
      at Parser.handlePacket (node_modules/pg-protocol/dist/parser.js:126:29)
      at Parser.parse (node_modules/pg-protocol/dist/parser.js:39:38)
      at Socket.<anonymous> (node_modules/pg-protocol/dist/index.js:11:42)
      at Socket.emit (domain.js:475:12)
      at addChunk (internal/streams/readable.js:293:12)
      at readableAddChunk (internal/streams/readable.js:267:9)
      at Socket.Readable.push (internal/streams/readable.js:206:10)
      at TCP.onStreamRead (internal/stream_base_commons.js:188:23)
      at TCP.callbackTrampoline (internal/async_hooks.js:130:17)

[nerdmax]

2.3.0 fixes the issue.

Just FYI, when I was testing this issue, I added .toSQL().toNative(); after insert(...) and it fixed the issue as well.

[salper]

FYI, this comes from the parameterize function in the client.

From

// json columns can have object in values.
if (isPlainObject(value)) {
  value = JSON.stringify(value);
}

To:

// json columns can have object in values.
if (isPlainObject(value) || Array.isArray(value)) {
  value = JSON.stringify(value);
}

I suppose that the intent of the fix was to allow JSON columns having arrays (as it's totally legit), but without introspection (or another hint), it's not possible to distinguish legit array columns from JSON columns with an array value.

[fxalgrain]

I get the same issue, by trying to update Knex from 2.3.0 to 2.4.0 due to the CVE.

As knex does know the column type, we implement on our side the JSON.stringify(value) if value is an array and if the column is JSONB

[matsko]

Same here. All JSON inserts of empty arrays fail.

[enchorb]

+1 on this, had to downgrade to 2.3.0.

A workaround for inserting arrays is like so {${array.join(',')}, but I rather not update this all across my codebase for a regression

[florianakos]

Is there any plan to release a fix for CVE reported in 2.3.0 without the regression introduced in 2.4.0?

[minh-hoang-trinh]

I submitted a new PR to add a check for array containing plain object rather than just do an Array.isArray check #5444

// check for plain object and array containing at least one plain object
if (isPlainObject(value) || (Array.isArray(value) && value.some(isPlainObject))) {
   value = JSON.stringify(value);
}

[caseywebdev]

@minh-hoang-trinh We use jsonb[] columns, which would still be broken with your suggestion.

Pretty sure the only option here is revert the change #5431

[minh-hoang-trinh]

@caseywebdev, I might have missed something, but IMHO, jsonb (not jsonb[]) should be able to store both json and json array (which is also a valid json) #5320 is a valid request.
(my take is that you should be able to use jsonb to store a json array)

my suggestion to change is to able to distinguish between primitive array value (such as string array, number array) versus plain object array (that should be store using JSON.stringify)

but I also noted that it might be better to serialize value base by column type, and not by the structure of input value

[caseywebdev]

We prefer to use jsonb[] when we know the top-level value must be an array. It gives us a guarantee that something like {"foo":"bar"} could never be stored in that column. It's true that jsonb could contain a top-level array, but it could also contain any other value.

Again, knex has serialized this way for so long now, I don't see another option besides reverting this breaking change.

[minh-hoang-trinh]

@caseywebdev I'm sorry, I have surely missed something (it's friday 👀)... Normally, the change suggested is just to JSON.stringify the input value if it's a plain object, or an array of plain object.

Can you please give me an example for case with json[] that still be broken ? I will try to add a test case to cover that.

[caseywebdev]

Our app has several instances of storing arrays of objects in json[] columns, which have always worked fine until the recent breaking change. Your proposal would continue to cause our case to be broken. Because this

select '[{"foo":"bar"}]'::json[]

is invalid

SQL Error [22P02]: ERROR: malformed array literal: "[{"foo":"bar"}]"
  Detail: "[" must introduce explicitly-specified array dimensions.
  Position: 8

The correct way to serialize an array with one item of {"foo": "bar"} into a json[] column is with this syntax (which knex has handled correctly forever)

select E'{"{\\"foo\\":\\"bar\\"}"}'::json[]

Hope that clears it up.

[minh-hoang-trinh]

thank @caseywebdev. Indeed it's now much clearer !

seems to me that insert json object (plain and array) tests are missing in integration tests. Maybe I can try to add some.

[kibertoad]

Released in 2.4.1