Skip to content

Commit d221796

Browse files
mtrezzamtrezza
committed
fix: Add configurable batch request sub-request limit
1 parent fb9ce56 commit d221796

File tree

9 files changed

+173
-1
lines changed

9 files changed

+173
-1
lines changed

‎spec/RequestComplexity.spec.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ describe('request complexity', () => {
147147
await reconfigureServer({});
148148
const config = Config.get('test');
149149
expect(config.requestComplexity).toEqual({
150+
batchRequestLimit: -1,
150151
includeDepth: -1,
151152
includeCount: -1,
152153
subqueryDepth: -1,

‎spec/SecurityCheckGroups.spec.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ describe('Security Check Groups', () => {
4444
queryDepth: 10,
4545
graphQLDepth: 50,
4646
graphQLFields: 200,
47+
batchRequestLimit: 50,
4748
};
4849
await reconfigureServer(config);
4950

‎spec/batch.spec.js‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,154 @@ describe('batch', () => {
594594
});
595595
}
596596

597+
describe('batch request size limit', () => {
598+
it('should reject batch request when sub-requests exceed batchRequestLimit', async () => {
599+
await reconfigureServer({
600+
requestComplexity: { batchRequestLimit: 2 },
601+
});
602+
await expectAsync(
603+
request({
604+
method: 'POST',
605+
url: 'http://localhost:8378/1/batch',
606+
headers,
607+
body: JSON.stringify({
608+
requests: [
609+
{ method: 'GET', path: '/1/classes/TestClass' },
610+
{ method: 'GET', path: '/1/classes/TestClass' },
611+
{ method: 'GET', path: '/1/classes/TestClass' },
612+
],
613+
}),
614+
})
615+
).toBeRejectedWith(
616+
jasmine.objectContaining({
617+
status: 400,
618+
data: jasmine.objectContaining({
619+
error: jasmine.stringContaining('3'),
620+
}),
621+
})
622+
);
623+
});
624+
625+
it('should allow batch request when sub-requests are within batchRequestLimit', async () => {
626+
await reconfigureServer({
627+
requestComplexity: { batchRequestLimit: 5 },
628+
});
629+
const result = await request({
630+
method: 'POST',
631+
url: 'http://localhost:8378/1/batch',
632+
headers,
633+
body: JSON.stringify({
634+
requests: [
635+
{ method: 'POST', path: '/1/classes/TestClass', body: { key: 'v1' } },
636+
{ method: 'POST', path: '/1/classes/TestClass', body: { key: 'v2' } },
637+
],
638+
}),
639+
});
640+
expect(result.data.length).toEqual(2);
641+
expect(result.data[0].success.objectId).toBeDefined();
642+
expect(result.data[1].success.objectId).toBeDefined();
643+
});
644+
645+
it('should allow batch request at exactly batchRequestLimit', async () => {
646+
await reconfigureServer({
647+
requestComplexity: { batchRequestLimit: 2 },
648+
});
649+
const result = await request({
650+
method: 'POST',
651+
url: 'http://localhost:8378/1/batch',
652+
headers,
653+
body: JSON.stringify({
654+
requests: [
655+
{ method: 'POST', path: '/1/classes/TestClass', body: { key: 'v1' } },
656+
{ method: 'POST', path: '/1/classes/TestClass', body: { key: 'v2' } },
657+
],
658+
}),
659+
});
660+
expect(result.data.length).toEqual(2);
661+
});
662+
663+
it('should not limit batch request when batchRequestLimit is -1 (disabled)', async () => {
664+
await reconfigureServer({
665+
requestComplexity: { batchRequestLimit: -1 },
666+
});
667+
const requests = Array.from({ length: 20 }, (_, i) => ({
668+
method: 'POST',
669+
path: '/1/classes/TestClass',
670+
body: { key: `v${i}` },
671+
}));
672+
const result = await request({
673+
method: 'POST',
674+
url: 'http://localhost:8378/1/batch',
675+
headers,
676+
body: JSON.stringify({ requests }),
677+
});
678+
expect(result.data.length).toEqual(20);
679+
});
680+
681+
it('should not limit batch request by default (no requestComplexity configured)', async () => {
682+
const requests = Array.from({ length: 20 }, (_, i) => ({
683+
method: 'POST',
684+
path: '/1/classes/TestClass',
685+
body: { key: `v${i}` },
686+
}));
687+
const result = await request({
688+
method: 'POST',
689+
url: 'http://localhost:8378/1/batch',
690+
headers,
691+
body: JSON.stringify({ requests }),
692+
});
693+
expect(result.data.length).toEqual(20);
694+
});
695+
696+
it('should bypass batchRequestLimit for master key requests', async () => {
697+
await reconfigureServer({
698+
requestComplexity: { batchRequestLimit: 2 },
699+
});
700+
const result = await request({
701+
method: 'POST',
702+
url: 'http://localhost:8378/1/batch',
703+
headers: {
704+
...headers,
705+
'X-Parse-Master-Key': 'test',
706+
},
707+
body: JSON.stringify({
708+
requests: [
709+
{ method: 'GET', path: '/1/classes/TestClass' },
710+
{ method: 'GET', path: '/1/classes/TestClass' },
711+
{ method: 'GET', path: '/1/classes/TestClass' },
712+
],
713+
}),
714+
});
715+
expect(result.data.length).toEqual(3);
716+
});
717+
718+
it('should include limit in error message when batch exceeds batchRequestLimit', async () => {
719+
await reconfigureServer({
720+
requestComplexity: { batchRequestLimit: 5 },
721+
});
722+
await expectAsync(
723+
request({
724+
method: 'POST',
725+
url: 'http://localhost:8378/1/batch',
726+
headers,
727+
body: JSON.stringify({
728+
requests: Array.from({ length: 10 }, () => ({
729+
method: 'GET',
730+
path: '/1/classes/TestClass',
731+
})),
732+
}),
733+
})
734+
).toBeRejectedWith(
735+
jasmine.objectContaining({
736+
status: 400,
737+
data: jasmine.objectContaining({
738+
error: jasmine.stringContaining('5'),
739+
}),
740+
})
741+
);
742+
});
743+
});
744+
597745
describe('subrequest path type validation', () => {
598746
it('rejects object path in batch subrequest with proper error instead of 500', async () => {
599747
await expectAsync(

‎src/Deprecator/Deprecations.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,11 @@ module.exports = [
7171
changeNewDefault: '200',
7272
solution: "Set 'requestComplexity.graphQLFields' to a positive integer appropriate for your app to limit the number of GraphQL field selections, or to '-1' to disable.",
7373
},
74+
{
75+
optionKey: 'requestComplexity.batchRequestLimit',
76+
changeNewDefault: '50',
77+
solution: "Set 'requestComplexity.batchRequestLimit' to a positive integer appropriate for your app to limit the number of sub-requests per batch request, or to '-1' to disable.",
78+
},
7479
{
7580
optionKey: 'enableProductPurchaseLegacyApi',
7681
changeNewKey: '',

‎src/Options/Definitions.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -674,6 +674,12 @@ module.exports.RateLimitOptions = {
674674
},
675675
};
676676
module.exports.RequestComplexityOptions = {
677+
batchRequestLimit: {
678+
env: 'PARSE_SERVER_REQUEST_COMPLEXITY_BATCH_REQUEST_LIMIT',
679+
help: 'Maximum number of sub-requests in a single batch request. Set to `-1` to disable. Default is `-1`.',
680+
action: parsers.numberParser('batchRequestLimit'),
681+
default: -1,
682+
},
677683
graphQLDepth: {
678684
env: 'PARSE_SERVER_REQUEST_COMPLEXITY_GRAPHQL_DEPTH',
679685
help: 'Maximum depth of GraphQL field selections. Set to `-1` to disable. Default is `-1`.',

‎src/Options/docs.js‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/Options/index.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,9 @@ export interface RequestComplexityOptions {
449449
:ENV: PARSE_SERVER_REQUEST_COMPLEXITY_GRAPHQL_FIELDS
450450
:DEFAULT: -1 */
451451
graphQLFields: ?number;
452+
/* Maximum number of sub-requests in a single batch request. Set to `-1` to disable. Default is `-1`.
453+
:DEFAULT: -1 */
454+
batchRequestLimit: ?number;
452455
}
453456

454457
export interface SecurityOptions {

‎src/Security/CheckGroups/CheckGroupServerConfig.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ class CheckGroupServerConfig extends CheckGroup {
145145
if (!rc) {
146146
throw 1;
147147
}
148-
const values = [rc.includeDepth, rc.includeCount, rc.subqueryDepth, rc.queryDepth, rc.graphQLDepth, rc.graphQLFields];
148+
const values = [rc.includeDepth, rc.includeCount, rc.subqueryDepth, rc.queryDepth, rc.graphQLDepth, rc.graphQLFields, rc.batchRequestLimit];
149149
if (values.some(v => v === -1)) {
150150
throw 1;
151151
}

‎src/batch.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,13 @@ async function handleBatch(router, req) {
6767
if (!Array.isArray(req.body?.requests)) {
6868
throw new Parse.Error(Parse.Error.INVALID_JSON, 'requests must be an array');
6969
}
70+
const batchRequestLimit = req.config?.requestComplexity?.batchRequestLimit ?? -1;
71+
if (batchRequestLimit > -1 && !req.auth?.isMaster && req.body.requests.length > batchRequestLimit) {
72+
throw new Parse.Error(
73+
Parse.Error.INVALID_JSON,
74+
`Batch request contains ${req.body.requests.length} sub-requests, which exceeds the limit of ${batchRequestLimit}.`
75+
);
76+
}
7077
for (const restRequest of req.body.requests) {
7178
if (!restRequest || typeof restRequest !== 'object' || typeof restRequest.path !== 'string') {
7279
throw new Parse.Error(Parse.Error.INVALID_JSON, 'batch request path must be a string');

0 commit comments

Comments
 (0)
⚡