-
Notifications
You must be signed in to change notification settings - Fork 13.1k
regression: incoming integration not working with new hono router #35772
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.
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
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #35772 +/- ##
========================================
Coverage 61.01% 61.01%
========================================
Files 3099 3099
Lines 73248 73248
Branches 16398 16398
========================================
+ Hits 44690 44693 +3
Misses 25534 25534
+ Partials 3024 3021 -3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
| const buffers = []; | ||
| for await (const chunk of this.request.body) { | ||
| buffers.push(chunk); | ||
| } | ||
| const content_raw = Buffer.concat(buffers).toString('utf8'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const MAX_PAYLOAD_SIZE = 10 * 1024 * 1024; // 10MB limit
const contentLength = parseInt(this.request.headers.get('content-length') || '0', 10);
if (contentLength > MAX_PAYLOAD_SIZE) {
throw new Error('Payload too large');
}
const buffers = [];
let totalSize = 0;
for await (const chunk of this.request.body) {
totalSize += chunk.length;
if (totalSize > MAX_PAYLOAD_SIZE) {
throw new Error('Payload too large');
}
buffers.push(chunk);
}
const content_raw = Buffer.concat(buffers).toString('utf8');The code buffers the entire request body into memory without size limits, which can lead to memory exhaustion for large payloads.
This issue appears in multiple locations:
- apps/meteor/app/integrations/server/api/api.js: Lines 91-95
- apps/meteor/app/integrations/server/api/api.js: Lines 328-328
Please implement size limits for request body processing to prevent memory exhaustion.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
f7769aa to
376f7a5
Compare
Introduced by #35078
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Description
This pull request addresses a regression issue where incoming integrations were not functioning correctly with the new hono router in the Rocket.Chat application. The changes are made in the following files:
apps/meteor/app/api/server/router.ts:
apps/meteor/app/integrations/server/api/api.js:
x-forwarded-protoheader.application/x-www-form-urlencodedpayloads that contain JSON.These changes aim to enhance the functionality and reliability of incoming integrations with the new router setup.