Prevent ERR_OUT_OF_RANGE errors when reading messageSize from buffer

[nbarnett]

Hi 👋 , this is a follow up to this PR from last year. We ran into an issue today where the const messageSize = buf.readUInt32BE(bufferOffset); line would cause an ERR_OUT_OF_RANGE error in about 1/5 executions of pgtyped. The error would always occur when parsing a file which referenced a large enum. The error began occurring when a change was merged to add two values to this enum.

My suspicion is that the additional enum value has caused the boundary of the TCP packet splitting to land in the middle of the bytes making up the messageSize integer. The fix that I have implemented is to first check that the buffer has enough data to read the indicator and messageSize, before attempting to do so.

I attempted to create a minimal reproduction, but was unable to. Running just the affected file resulted in the error occurring only about 1/20 executions, and I wasn't able to reproduce the error at all when I attempted to simplify the DB schema to just the large enum and associated tables.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
pgtyped	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 29, 2024 5:42am

[benjie]

We're facing the same issue, and can confirm that applying the proposed fix via pnpm patch seems to fix it:

diff --git a/lib/protocol.js b/lib/protocol.js
index 7bef96597d29825a79786d73418af79e15b1f6eb..b771c2bc34df0383fafba8d6071f652ff94c4f56 100644
--- a/lib/protocol.js
+++ b/lib/protocol.js
@@ -49,6 +49,16 @@ export const parseSimpleType = (type, buf, offset, offsetEnd) => {
 const errorResponseMessageIndicator = pgMessages.errorResponse.indicator.charCodeAt(0);
 export const parseMessage = (message, buf, messageOffset = 0) => {
     let bufferOffset = messageOffset;
+
+    // Check if we have enough data to read the indicator and message size
+    // The + 5 is made up of 1 byte for readInt8 and 4 bytes for readUInt32BE
+    if (bufferOffset + 5 > buf.length) {
+      return {
+        type: "IncompleteMessageError",
+        messageName: message.name,
+      };
+    }
+
     const indicator = buf.readInt8(bufferOffset);
     const expectedIndicator = message.indicator.charCodeAt(0);
     const isUnexpectedErrorMessage = indicator === errorResponseMessageIndicator &&

[benjie]

(We can also confirm it's incredibly hard to reproduce this across environments, but I was "lucky" enough to hit it 9 times out of 10 on our project.)

[ml-mave]

I can add another confirmation. We fixed this internally by adding a similar check after the first readInt8 call because it shouldn't be possible to enter this function if the buffer has no bytes left at all (because of the checks in the calling function). No more errors after the fix so far. We were too slow at creating a PR. :)

[adelsz]

Thanks for the PR @nbarnett!
Looks good to me. In the long term it would be great to migrate to 3rd party PG protocol library but that is a much bigger task.