regression: parse urlencoded body as string when there is no specific key(payload) #35823
Conversation
|
Looks like this PR is ready to merge! 🎉 |
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:
|
|
MarcosSpessatto
changed the title
![kody-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
| msg: 'Body received as application/x-www-form-urlencoded without the "payload" key, parsed as string', | ||
| content, | ||
| }); | ||
| c.set('bodyParams-override', JSON.parse(content)); |
There was a problem hiding this comment.
try {
const singleKey = Object.keys(body)[0];
const valueToParse = body[singleKey];
c.set('bodyParams-override', JSON.parse(valueToParse));
} catch (parseError) {
incomingLogger.error({ msg: 'Failed to parse form field value as JSON', key: Object.keys(body)[0], error: parseError });
c.body(JSON.stringify({ success: false, error: 'Invalid JSON in form field value' }), 400);
return; // Stop processing here
}Parsing the raw form content (content) as JSON is likely incorrect, as it should be the value of the single form field that is parsed instead.
This issue appears in multiple locations:
- apps/meteor/app/integrations/server/api/api.js: Lines 343-343
- apps/meteor/app/integrations/server/api/api.js: Lines 343-343
Please modify the code to parse the value of the single form field as JSON instead of the raw form content to avoid potential parsing errors.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const content_raw = Buffer.concat(buffers).toString('utf8'); | ||
| const protocol = `${this.request.headers.get('x-forwarded-proto')}:` || 'http:'; | ||
| const url = new URL(this.request.url, `${protocol}//${this.request.headers.host}`); | ||
| const url = new URL(this.request.url, `${protocol}//${this.request.headers.get('host')}`); |
There was a problem hiding this comment.
const host = this.request.headers.get('host');
if (!host) {
// Option 1: Throw an error if host is required
throw new Error('Host header is missing');
// Option 2: Handle appropriately if a fallback isn't possible
// return API.v1.failure('Missing Host header');
}
const url = new URL(this.request.url, `${protocol}//${host}`);The this.request.headers.get('host') call might return null, leading to potential errors when constructing a URL with an invalid host part.
This issue appears in multiple locations:
- apps/meteor/app/integrations/server/api/api.js: Lines 97-97
- apps/meteor/app/integrations/server/api/api.js: Lines 97-97
Please add validation for the 'host' header to ensure it is present and handle cases where it is missing to prevent errors in URL construction.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| c.set('bodyParams-override', JSON.parse(body.payload)); | ||
| return next(); | ||
| } | ||
| incomingLogger.debug({ | ||
| msg: 'Body received as application/x-www-form-urlencoded without the "payload" key, parsed as string', | ||
| content, | ||
| }); | ||
| c.set('bodyParams-override', JSON.parse(content)); |
There was a problem hiding this comment.
try {
const parsed = JSON.parse(body.payload);
c.set('bodyParams-override', parsed);
return next();
} catch (e) {
incomingLogger.error({ msg: 'Failed to parse payload as JSON', error: e });
c.body(JSON.stringify({ success: false, error: 'Invalid JSON payload' }), 400);
return;
}The JSON.parse calls are within an outer try...catch block, which may lead to generic error handling and less granular error logging.
This issue appears in multiple locations:
- apps/meteor/app/integrations/server/api/api.js: Lines 336-343
- apps/meteor/app/integrations/server/api/api.js: Lines 336-343
Please add specifictry...catchblocks around eachJSON.parsecall to enable more granular error handling and logging.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #35823 +/- ##
========================================
Coverage 61.16% 61.16%
========================================
Files 2971 2971
Lines 70839 70839
Branches 16185 16185
========================================
+ Hits 43328 43330 +2
+ Misses 24562 24560 -2
Partials 2949 2949
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:
|
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:
|
|
ggazzo
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
This pull request addresses a regression issue related to the parsing of
application/x-www-form-urlencodedrequest bodies. The changes focus on refining the handling of incoming webhook requests and improving error management in theparseBodyParamsmethod.Key changes include:
apps/meteor/app/api/server/router.ts, the error handling within theparseBodyParamsmethod has been updated. Now, when an error occurs during parsing, the method returns an empty object instead of theoverrideBodyParams.apps/meteor/app/integrations/server/api/api.js, the construction of request URLs for incoming webhooks has been adjusted. Additionally, the logic for parsingapplication/x-www-form-urlencodedrequest bodies has been refined, especially for those containing a JSON payload within a form field.These updates aim to enhance the robustness of the URL-encoded body parsing process, ensuring that errors are handled gracefully and that the request data is accurately interpreted.