fuzz: draw the binary result format only for types whose bytes are valid utf8

Author: nigrosimone

.github/workflows/ci.yml

@@ -101,11 +101,11 @@ jobs:
   # into a test: replay it locally with `node fuzz/<tool>.js --seed N --rounds 1`.
   #
   # What the first runs found, each fixed in its own pull request: #3776 (a copyData chunk
-  # changing under whoever kept it), #3777 (binary results losing every byte utf8 cannot
-  # carry), #3778 (pg-native reporting NaN and '' for a missing row count or command), #3780
-  # (native errors without detail and hint), #3781 (a named empty statement failing from its
-  # second run). And one it cannot fix here: a Buffer parameter reaches libpq as a C string,
-  # #980, so the native arm draws none.
+  # changing under whoever kept it), #3778 (pg-native reporting NaN and '' for a missing row
+  # count or command), #3780 (native errors without detail and hint), #3781 (a named empty
+  # statement failing from its second run). And two it does not fix here: a binary result is
+  # decoded as utf8 first, #3496, so the binary arm draws only types whose bytes are valid utf8;
+  # a Buffer parameter reaches libpq as a C string, #980, so the native arm draws none.
   fuzz:
     timeout-minutes: 15
     needs: lint

fuzz/README.md

@@ -21,6 +21,7 @@ node fuzz/modes.js --no-shrink         # print the round as drawn
 The queries come from `queries.js`: selects over `generate_series` with a column per type family,
 sometimes as a parameter instead of a literal, writes on a temp table, and statements that fail
 on purpose, some of them only after rows were sent. What the binary arm can compare is limited to
-the types pg-types has a binary parser for, the `binary` flag of each type says which. The native
+the types pg-types has a binary parser for and whose bytes are valid utf8, because the parser
+decodes a binary value as utf8 first (#3496). The `binary` flag of each type says which. The native
 arm draws no Buffer parameter: pg-native hands it to libpq as a C string, cut at its first zero
 byte (#980), and the fix for that is in node-libpq.

fuzz/queries.js

@@ -59,14 +59,16 @@ const hex = (rng) => `\\x${bytes(rng, 8).toString('hex')}`
 
 // one entry per type family. `literal` is a SQL expression, `param` a JS value pg sends for it,
 // `binary` says whether the binary result format gives the same JS value as the text one, which
-// is only true for the types pg-types has a binary parser for, or decodes as utf8 anyway
+// is only true for the types pg-types has a binary parser for, or decodes as utf8 anyway. The
+// parser decodes a binary value as utf8 before pg-types sees it (#3496), so a type whose bytes
+// can be invalid utf8 (numbers, oid, an array header) is false too
 const TYPES = [
-  { type: 'int2', binary: true, literal: (rng) => `${ints(rng, 16)}::int2`, param: (rng) => ints(rng, 16) },
-  { type: 'int4', binary: true, literal: (rng) => `${ints(rng, 32)}::int4`, param: (rng) => ints(rng, 32) },
-  { type: 'int8', binary: true, literal: (rng) => `${ints(rng, 53)}::int8`, param: (rng) => String(ints(rng, 53)) },
+  { type: 'int2', binary: false, literal: (rng) => `${ints(rng, 16)}::int2`, param: (rng) => ints(rng, 16) },
+  { type: 'int4', binary: false, literal: (rng) => `${ints(rng, 32)}::int4`, param: (rng) => ints(rng, 32) },
+  { type: 'int8', binary: false, literal: (rng) => `${ints(rng, 53)}::int8`, param: (rng) => String(ints(rng, 53)) },
   {
     type: 'float8',
-    binary: true,
+    binary: false,
     literal: (rng) => `${quote(String(floats(rng)))}::float8`,
     param: (rng) => floats(rng),
   },
@@ -136,7 +138,7 @@ const TYPES = [
   { type: 'uuid', binary: false, literal: () => `${quote(uuid())}::uuid`, param: () => uuid() },
   {
     type: 'oid',
-    binary: true,
+    binary: false,
     literal: (rng) => `${int(rng, 0, 2147483647)}::oid`,
     param: (rng) => int(rng, 0, 2147483647),
   },
@@ -160,7 +162,7 @@ const TYPES = [
   },
   {
     type: 'text[]',
-    binary: true,
+    binary: false,
     literal: (rng) => `${quote(arrayLiteral(arrayOf(rng, texts)))}::text[]`,
     param: (rng) => arrayOf(rng, texts),
   },
@@ -199,7 +201,7 @@ const TYPES = [
   { type: 'unknown', binary: true, literal: (rng) => quote(texts(rng)) },
   { type: 'null', binary: true, literal: () => 'NULL', param: () => null },
   // something that changes with the row, so a result is not one value repeated
-  { type: 'row', binary: true, literal: () => 'g' },
+  { type: 'row', binary: false, literal: () => 'g' },
   { type: 'rowtext', binary: true, literal: () => `'r' || g::text` },
 ]
 

fuzz/wire.js

@@ -245,20 +245,6 @@ const observed = (parsed, expect) => {
   return out
 }
 
-// what each message must parse to, in the sequence it is in: a column the last row description
-// declared binary comes back as its bytes, the others as text
-const expectations = (messages) => {
-  let binaryColumns = []
-  return messages.map(({ expect }) => {
-    if (expect.name === 'rowDescription') binaryColumns = expect.fields.map((f) => f.format === 'binary')
-    if (expect.name !== 'dataRow') return expect
-    const fields = expect.fields.map((field, i) =>
-      field !== null && binaryColumns[i] ? Buffer.from(field, 'utf8') : field
-    )
-    return { ...expect, fields }
-  })
-}
-
 const parseAll = (chunks, expects) => {
   const parser = new Parser()
   const out = []
@@ -268,7 +254,7 @@ const parseAll = (chunks, expects) => {
 
 const run = async (plan) => {
   const whole = Buffer.concat(plan.messages.map((m) => m.bytes))
-  const expects = expectations(plan.messages)
+  const expects = plan.messages.map((m) => m.expect)
   const chunks = []
   let from = 0
   for (const at of [...plan.cuts, whole.length]) {