Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bug: trpc/server node-http adapter fails to read large POST body #5725

Closed
1 task done
jcarrus opened this issue May 16, 2024 · 1 comment · Fixed by #5724
Closed
1 task done

bug: trpc/server node-http adapter fails to read large POST body #5725

jcarrus opened this issue May 16, 2024 · 1 comment · Fixed by #5724
Labels
🐛 bug Something isn't working

Comments

@jcarrus
Copy link
Contributor

jcarrus commented May 16, 2024 •
edited by polar-sh bot

Provide environment information

  System:
    OS: Linux 6.1 Manjaro Linux
    CPU: (24) x64 13th Gen Intel(R) Core(TM) i7-13700
    Memory: 25.82 GB / 62.54 GB
    Container: Yes
    Shell: 5.2.26 - /bin/bash
  Binaries:
    Node: 20.12.1 - ~/.nvm/versions/node/v20.12.1/bin/node
    npm: 10.5.0 - ~/.nvm/versions/node/v20.12.1/bin/npm
    pnpm: 8.15.5 - ~/.nvm/versions/node/v20.12.1/bin/pnpm
    bun: 1.1.8 - /usr/bin/bun
  npmPackages:
    typescript: ^5.4.5 => 5.4.5

Describe the bug

I'm using trpc to POST large JSON objects to my backend. I've noticed that large bodies are truncated since the rewrite in #5684. In my case, this results in an error of:

[
  {
    "error": {
      "message": "Unterminated string in JSON at position 16375",
      "code": -32600,
      "data": {
        "code": "BAD_REQUEST",
        "httpStatus": 400,
        "stack": "TRPCError: Unterminated string in JSON at position 16375\n    at file:///app/node_modules/@trpc/server/dist/unstable-core-do-not-import/http/contentType.mjs:23:27\n    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)\n    at async Object.read (file:///app/node_modules/@trpc/server/dist/unstable-core-do-not-import/http/contentType.mjs:30:21)\n    at async Object.getRawInput (file:///app/node_modules/@trpc/server/dist/unstable-core-do-not-import/http/contentType.mjs:89:40)\n    at async inputValidatorMiddleware (file:///app/node_modules/@trpc/server/dist/unstable-core-do-not-import/middleware.mjs:42:26)\n    at async callRecursive (file:///app/node_modules/@trpc/server/dist/unstable-core-do-not-import/procedureBuilder.mjs:152:32)\n    at async procedure (file:///app/node_modules/@trpc/server/dist/unstable-core-do-not-import/procedureBuilder.mjs:182:24)\n    at async file:///app/node_modules/@trpc/server/dist/unstable-core-do-not-import/http/resolveResponse.mjs:135:30\n    at async Promise.all (index 0)\n    at async resolveResponse (file:///app/node_modules/@trpc/server/dist/unstable-core-do-not-import/http/resolveResponse.mjs:178:37)\n    at async file:///app/node_modules/@trpc/server/dist/adapters/node-http/nodeHTTPRequestHandler.mjs:33:26\n    at async file:///app/node_modules/@trpc/server/dist/adapters/express.mjs:7:9",
        "path": "trpcMethodName"
      }
    }
  }
]

In doing some testing, I believe that the issue with the current trpc server code is that the incomingMessageToBodyStream function is not properly reading the full stream.

trpc/packages/server/src/adapters/node-http/incomingMessageToRequest.ts

Lines 23 to 50 in 3dc8f3b

const stream = new ReadableStream<Value>({
start(c) {
controller = c;
},
async pull(c) {
const chunk: Value = req.read();
if (chunk) {
size += chunk.length;
}
if (maxBodySize !== null && size > maxBodySize) {
controller.error(
new TRPCError({
code: 'PAYLOAD_TOO_LARGE',
}),
);
return;
}
if (chunk === null) {
c.close();
return;
}
controller.enqueue(chunk);
},
cancel() {
req.destroy();
},
});

The issue occurs when the pull method is called before there is anything new to read, but the body has not been fully parsed. This seems to happen either when a highwater mark is reached on an internal stream or there is a break in the delivery of the network packets.

When the pull method is called, but read() returns null, the controller.enqueue function is not called, which causes the ReadableStream to stop calling pull. This wasn't obvious to me either, but is clearly stated on the MDN page:

pull (optional)
This method, also defined by the developer, will be called repeatedly when the stream's internal queue of chunks is not full, up until it reaches its high water mark. If pull() returns a promise, then it won't be called again until that promise fulfills; if the promise rejects, the stream will become errored. The controller parameter passed to this method is a ReadableStreamDefaultController or a ReadableByteStreamController, depending on the value of the type property. This can be used by the developer to control the stream as more chunks are fetched. This function will not be called until start() successfully completes. Additionally, it will only be called repeatedly if it enqueues at least one chunk or fulfills a BYOB request; a no-op pull() implementation will not be continually called.

(bolded text is mine)

Link to reproduction

#5724

To reproduce

Checkout the PR (#5724) and confirm that the new test fails without the accompanying bugfix, but all tests pass with the fix.

Additional information

No response

👨‍👧‍👦 Contributing

  • 🙋‍♂️ Yes, I'd be down to file a PR fixing this bug!

Funding

  • You can sponsor this specific effort via a Polar.sh pledge below
  • We receive the pledge once the issue is completed & verified
Fund with Polar
@jcarrus jcarrus mentioned this issue May 16, 2024
Merged
3 tasks
@KATT KATT added 🐛 bug Something isn't working and removed 🐛 bug: unconfirmed labels May 16, 2024
@github-actions GitHub Actions
Copy link

This issue has been locked because we are very unlikely to see comments on closed issues. If you are running into a similar issue, please create a new issue. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
🐛 bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(server): handle large POST bodies in node-http jcarrus/trpc
2 participants
@KATT @jcarrus