Skip to content

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.

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

Merged
ggazzo merged 2 commits into from
Apr 15, 2025
Merged

regression: incoming integration not working with new hono router #35772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
21ad3b6
regression: incoming integration not working with new hono router
Apr 10, 2025
376f7a5
test: add test to ensure behavior
Apr 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions apps/meteor/app/api/server/router.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ export class Router<
}

private async parseBodyParams(request: HonoRequest, overrideBodyParams: Record<string, any> = {}) {
if (Object.keys(overrideBodyParams).length !== 0) {
return overrideBodyParams;
}

try {
let parsedBody = {};
const contentType = request.header('content-type');
Expand All @@ -151,14 +155,14 @@ export class Router<
}
// This is necessary to keep the compatibility with the previous version, otherwise the bodyParams will be an empty string when no content-type is sent
if (parsedBody === '') {
return { ...overrideBodyParams };
return {};
}

if (Array.isArray(parsedBody)) {
return parsedBody;
}

return { ...parsedBody, ...overrideBodyParams };
return { ...parsedBody };
// eslint-disable-next-line no-empty
} catch {}

Expand Down
24 changes: 14 additions & 10 deletions apps/meteor/app/integrations/server/api/api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,25 +87,29 @@ async function executeIntegrationRest() {
};

const scriptEngine = getEngine(this.request.integration);

if (scriptEngine.integrationHasValidScript(this.request.integration)) {
this.request.setEncoding('utf8');
const content_raw = this.request.read();
const buffers = [];
for await (const chunk of this.request.body) {
buffers.push(chunk);
}
const content_raw = Buffer.concat(buffers).toString('utf8');
Comment on lines +91 to +95
Copy link

@kody-ai kody-ai bot Apr 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Performance and Optimization critical

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.

​

​

const protocol = `${this.request.headers.get('x-forwarded-proto')}:` || 'http:';
const url = new URL(this.request.url, `${protocol}//${this.request.headers.host}`);

const request = {
url: {
hash: this.request._parsedUrl.hash,
search: this.request._parsedUrl.search,
hash: url.hash,
search: url.search,
query: this.queryParams,
pathname: this.request._parsedUrl.pathname,
path: this.request._parsedUrl.path,
pathname: url.pathname,
path: url.path,
},
url_raw: this.request.url,
url_params: this.urlParams,
content: this.bodyParams,
content_raw,
headers: this.request.headers,
body: this.request.body,
headers: Object.fromEntries(this.request.headers.entries()),
body: this.bodyParams,
user: {
_id: this.user._id,
name: this.user.name,
Expand Down Expand Up @@ -321,7 +325,7 @@ const middleware = async (c, next) => {
}

try {
const body = await (req.header('content-type')?.includes('application/json') ? req.raw.clone().json() : req.raw.clone().text());
const body = Object.fromEntries(new URLSearchParams(await req.raw.clone().text()));
if (!body || typeof body !== 'object' || !('payload' in body) || Object.keys(body).length !== 1) {
return next();
}
Expand Down
65 changes: 65 additions & 0 deletions apps/meteor/tests/end-to-end/api/incoming-integrations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,71 @@ describe('[Incoming Integrations]', () => {
});
});
});

it('should send a message if the payload is a application/x-www-form-urlencoded JSON AND the integration has a valid script', async () => {
const payload = { msg: `Message as x-www-form-urlencoded JSON sent successfully at #${Date.now()}` };
let withScript: IIntegration | undefined;

await updatePermission('manage-incoming-integrations', ['admin']);
await request
.post(api('integrations.create'))
.set(credentials)
.send({
type: 'webhook-incoming',
name: 'Incoming test with script',
enabled: true,
alias: 'test',
username: 'rocket.cat',
scriptEnabled: true,
overrideDestinationChannelEnabled: false,
channel: '#general',
script: `
class Script {
process_incoming_request({ request }) {
return {
content:{
text: request.content.text
}
};
}
}
`,
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('integration').and.to.be.an('object');
withScript = res.body.integration;
});

if (!withScript) {
throw new Error('Integration not created');
}

await request
.post(`/hooks/${withScript._id}/${withScript.token}`)
.set('Content-Type', 'application/x-www-form-urlencoded')
.send(`payload=${JSON.stringify(payload)}`)
.expect(200)
.expect(async () => {
return request
.get(api('channels.messages'))
.set(credentials)
.query({
roomId: 'GENERAL',
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('messages').and.to.be.an('array');
expect(!!(res.body.messages as IMessage[]).find((m) => m.msg === payload.msg)).to.be.true;
});
});

await removeIntegration(withScript._id, 'incoming');
});
});

describe('[/integrations.history]', () => {
Expand Down
Loading