mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-21 09:32:24 +00:00
🗜️ fix: Bound Conversation Page Size (#15576)
`GET /api/convos` read its page size as `parseInt(req.query.limit, 10) || 25` and handed it to `getConvosByCursor` unclamped, where it becomes `.limit(limit + 1)`. `?limit=-1` therefore issued `.limit(0)`, which MongoDB reads as no limit at all, and the route returned the caller's entire conversation list in one response; `?limit=999999999` forced the same unbounded fetch. `attachSharedFlags` then amplified it with a second query whose `$in` carried every returned id. Every other list route already clamps, so the fix reuses that shape: the projects handler's local `normalizeLimit` moves to `packages/api/src/utils/query` and both routes share it, and `getConvosByCursor` bounds its own page size so the invariant holds for any caller.
This commit is contained in:
parent
374ed8df14
commit
78b320cbbc
9 changed files with 246 additions and 22 deletions
|
|
@ -36,6 +36,15 @@ module.exports = {
|
|||
}
|
||||
return false;
|
||||
}),
|
||||
/** Mirrors the real helper so the route's page-size clamping is exercised. */
|
||||
normalizeLimit: jest.fn((value, { fallback = 25, max = 100 } = {}) => {
|
||||
const raw = Array.isArray(value) ? value[0] : value;
|
||||
const limit = parseInt(typeof raw === 'string' ? raw : '', 10);
|
||||
if (!Number.isFinite(limit)) {
|
||||
return fallback;
|
||||
}
|
||||
return Math.min(Math.max(limit, 1), max);
|
||||
}),
|
||||
resolveImportMaxFileSize: jest.fn(() => 262144000),
|
||||
createAxiosInstance: jest.fn(() => ({
|
||||
get: jest.fn(),
|
||||
|
|
|
|||
|
|
@ -1417,6 +1417,73 @@ describe('Convos Routes', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('GET / limit clamping', () => {
|
||||
const { getConvosByCursor } = require('~/models');
|
||||
|
||||
beforeEach(() => {
|
||||
getConvosByCursor.mockResolvedValue({ conversations: [], nextCursor: null });
|
||||
});
|
||||
|
||||
/** `parseInt('-1') || 25` kept the -1, and `.limit(-1 + 1)` is `.limit(0)`, which
|
||||
* MongoDB reads as "no limit" — one request drained the caller's whole list. */
|
||||
it('clamps a negative limit instead of forwarding it', async () => {
|
||||
const response = await request(app).get('/api/convos').query({ limit: '-1' });
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(getConvosByCursor).toHaveBeenCalledWith(
|
||||
'test-user-123',
|
||||
expect.objectContaining({ limit: 1 }),
|
||||
);
|
||||
});
|
||||
|
||||
it('caps an oversized limit at the page ceiling', async () => {
|
||||
const response = await request(app).get('/api/convos').query({ limit: '999999999' });
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(getConvosByCursor).toHaveBeenCalledWith(
|
||||
'test-user-123',
|
||||
expect.objectContaining({ limit: 100 }),
|
||||
);
|
||||
});
|
||||
|
||||
it('raises a zero limit to a single row', async () => {
|
||||
const response = await request(app).get('/api/convos').query({ limit: '0' });
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(getConvosByCursor).toHaveBeenCalledWith(
|
||||
'test-user-123',
|
||||
expect.objectContaining({ limit: 1 }),
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['absent', undefined],
|
||||
['non-numeric', 'all'],
|
||||
])('falls back to the default page size when the limit is %s', async (_label, limit) => {
|
||||
const response = await request(app)
|
||||
.get('/api/convos')
|
||||
.query(limit === undefined ? {} : { limit });
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(getConvosByCursor).toHaveBeenCalledWith(
|
||||
'test-user-123',
|
||||
expect.objectContaining({ limit: 25 }),
|
||||
);
|
||||
});
|
||||
|
||||
/** Express parses a repeated key into an array, which `parseInt` silently reads as
|
||||
* its first element's digits rather than rejecting. */
|
||||
it('clamps a repeated limit parameter', async () => {
|
||||
const response = await request(app).get('/api/convos?limit=999999&limit=5');
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(getConvosByCursor).toHaveBeenCalledWith(
|
||||
'test-user-123',
|
||||
expect.objectContaining({ limit: 100 }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /archive', () => {
|
||||
it('should archive a conversation successfully', async () => {
|
||||
const mockConversationId = 'conv-123';
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ const express = require('express');
|
|||
const { sleep } = require('@librechat/agents');
|
||||
const {
|
||||
isEnabled,
|
||||
normalizeLimit,
|
||||
deleteAgentCheckpoints,
|
||||
createArchiveAllHandler,
|
||||
createSubagentActivityStreamHandler,
|
||||
|
|
@ -141,7 +142,7 @@ const isValidProjectFilter = (projectId) =>
|
|||
!projectId || projectId === 'unassigned' || /^[a-f\d]{24}$/i.test(projectId);
|
||||
|
||||
router.get('/', async (req, res) => {
|
||||
const limit = parseInt(req.query.limit, 10) || 25;
|
||||
const limit = normalizeLimit(req.query.limit);
|
||||
const cursor = req.query.cursor;
|
||||
const isArchived = isEnabled(req.query.isArchived);
|
||||
const pinned = isEnabled(req.query.pinned);
|
||||
|
|
|
|||
|
|
@ -9,6 +9,8 @@ import type {
|
|||
} from '@librechat/data-schemas';
|
||||
import type { Request, Response } from 'express';
|
||||
|
||||
import { normalizeLimit, queryString } from '~/utils';
|
||||
|
||||
const PROJECT_NOT_FOUND = 'Project not found';
|
||||
const CONVERSATION_NOT_FOUND = 'Conversation not found';
|
||||
|
||||
|
|
@ -37,27 +39,9 @@ type ProjectHandlerDependencies = Pick<
|
|||
|
||||
const getUserId = (req: ProjectRequest): string => req.user?.id ?? req.user?._id?.toString() ?? '';
|
||||
|
||||
const queryString = (value: Request['query'][string]): string | undefined => {
|
||||
if (typeof value === 'string') {
|
||||
return value;
|
||||
}
|
||||
if (Array.isArray(value)) {
|
||||
return queryString(value[0]);
|
||||
}
|
||||
return undefined;
|
||||
};
|
||||
|
||||
const normalizeString = (value: string | null | undefined): string =>
|
||||
typeof value === 'string' ? value.trim() : '';
|
||||
|
||||
const normalizeLimit = (value: Request['query'][string]): number => {
|
||||
const limit = parseInt(queryString(value) ?? '', 10);
|
||||
if (!Number.isFinite(limit)) {
|
||||
return 25;
|
||||
}
|
||||
return Math.min(Math.max(limit, 1), 100);
|
||||
};
|
||||
|
||||
const normalizeSortBy = (value: Request['query'][string]): ChatProjectSortBy | undefined => {
|
||||
const sortBy = queryString(value);
|
||||
return PROJECT_SORT_FIELDS.has(sortBy as ChatProjectSortBy)
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ export * from './oidc';
|
|||
export * from './openid';
|
||||
export * from './promise';
|
||||
export * from './proxy';
|
||||
export * from './query';
|
||||
export * from './ports';
|
||||
export * from './sanitizeTitle';
|
||||
export * from './text';
|
||||
|
|
|
|||
56
packages/api/src/utils/query.spec.ts
Normal file
56
packages/api/src/utils/query.spec.ts
Normal file
|
|
@ -0,0 +1,56 @@
|
|||
import { DEFAULT_PAGE_LIMIT, MAX_PAGE_LIMIT, normalizeLimit, queryString } from './query';
|
||||
|
||||
describe('queryString', () => {
|
||||
it('returns a string value unchanged', () => {
|
||||
expect(queryString('dev')).toBe('dev');
|
||||
});
|
||||
|
||||
it('reads the first entry of a repeated parameter', () => {
|
||||
expect(queryString(['dev', 'main'])).toBe('dev');
|
||||
});
|
||||
|
||||
it.each([[undefined], [{ nested: 'value' }], [[]]])('returns undefined for %p', (value) => {
|
||||
expect(queryString(value as Parameters<typeof queryString>[0])).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('normalizeLimit', () => {
|
||||
it('keeps a limit inside the allowed range', () => {
|
||||
expect(normalizeLimit('40')).toBe(40);
|
||||
});
|
||||
|
||||
/** `.limit(0)` is "no limit" in MongoDB, so a negative page size returned everything. */
|
||||
it.each([
|
||||
['-1', 1],
|
||||
['-5', 1],
|
||||
['0', 1],
|
||||
])('raises %s to the minimum page size', (value, expected) => {
|
||||
expect(normalizeLimit(value)).toBe(expected);
|
||||
});
|
||||
|
||||
it.each([['999999999'], ['101']])('caps %s at the maximum page size', (value) => {
|
||||
expect(normalizeLimit(value)).toBe(MAX_PAGE_LIMIT);
|
||||
});
|
||||
|
||||
it.each([[undefined], ['all'], [''], [{ limit: '10' }]])(
|
||||
'falls back to the default for %p',
|
||||
(value) => {
|
||||
expect(normalizeLimit(value as Parameters<typeof normalizeLimit>[0])).toBe(
|
||||
DEFAULT_PAGE_LIMIT,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it('reads the first entry of a repeated parameter before clamping', () => {
|
||||
expect(normalizeLimit(['999999', '5'])).toBe(MAX_PAGE_LIMIT);
|
||||
});
|
||||
|
||||
it('honors caller-supplied bounds', () => {
|
||||
expect(normalizeLimit('500', { max: 1000 })).toBe(500);
|
||||
expect(normalizeLimit('nope', { fallback: 10 })).toBe(10);
|
||||
});
|
||||
|
||||
it('truncates a fractional limit', () => {
|
||||
expect(normalizeLimit('7.9')).toBe(7);
|
||||
});
|
||||
});
|
||||
32
packages/api/src/utils/query.ts
Normal file
32
packages/api/src/utils/query.ts
Normal file
|
|
@ -0,0 +1,32 @@
|
|||
import type { Request } from 'express';
|
||||
|
||||
type QueryValue = Request['query'][string];
|
||||
|
||||
export const DEFAULT_PAGE_LIMIT = 25;
|
||||
export const MAX_PAGE_LIMIT = 100;
|
||||
|
||||
/** Express parses a repeated `?limit=a&limit=b` into an array, which `parseInt` reads as its first element's digits. */
|
||||
export const queryString = (value: QueryValue): string | undefined => {
|
||||
if (typeof value === 'string') {
|
||||
return value;
|
||||
}
|
||||
if (Array.isArray(value)) {
|
||||
return queryString(value[0]);
|
||||
}
|
||||
return undefined;
|
||||
};
|
||||
|
||||
/**
|
||||
* Clamps a page size from user input. An unclamped limit reaches Mongo's `.limit()`,
|
||||
* where a negative value means "one batch" and `0` means "no limit at all".
|
||||
*/
|
||||
export const normalizeLimit = (
|
||||
value: QueryValue,
|
||||
{ fallback = DEFAULT_PAGE_LIMIT, max = MAX_PAGE_LIMIT }: { fallback?: number; max?: number } = {},
|
||||
): number => {
|
||||
const limit = parseInt(queryString(value) ?? '', 10);
|
||||
if (!Number.isFinite(limit)) {
|
||||
return fallback;
|
||||
}
|
||||
return Math.min(Math.max(limit, 1), max);
|
||||
};
|
||||
|
|
@ -3665,6 +3665,72 @@ describe('Conversation Operations', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('getConvosByCursor page size bounds', () => {
|
||||
const TOTAL = 105;
|
||||
const user = 'page-size-user';
|
||||
|
||||
beforeEach(async () => {
|
||||
const baseTime = new Date('2026-06-01T00:00:00.000Z').getTime();
|
||||
await Conversation.collection.insertMany(
|
||||
Array.from({ length: TOTAL }, (_, index) => ({
|
||||
conversationId: uuidv4(),
|
||||
user,
|
||||
title: `Conversation ${index}`,
|
||||
endpoint: EModelEndpoint.openAI,
|
||||
expiredAt: null,
|
||||
isArchived: false,
|
||||
createdAt: new Date(baseTime + index * 1000),
|
||||
updatedAt: new Date(baseTime + index * 1000),
|
||||
})),
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await Conversation.deleteMany({ user });
|
||||
});
|
||||
|
||||
/** `.limit(limit + 1)` turned a -1 page size into `.limit(0)`, which MongoDB reads as
|
||||
* "no limit" — the query returned the caller's entire collection in one response. */
|
||||
it.each([
|
||||
['negative', -1],
|
||||
['zero', 0],
|
||||
['oversized', 999999999],
|
||||
])('bounds a %s page size', async (_label, limit) => {
|
||||
const result = await getConvosByCursor(user, { limit });
|
||||
|
||||
expect(result.conversations.length).toBeLessThanOrEqual(100);
|
||||
expect(result.conversations.length).toBeGreaterThan(0);
|
||||
expect(result.nextCursor).not.toBeNull();
|
||||
});
|
||||
|
||||
it('caps an oversized page size at the ceiling and still pages the remainder', async () => {
|
||||
const first = await getConvosByCursor(user, { limit: Number.MAX_SAFE_INTEGER });
|
||||
|
||||
expect(first.conversations).toHaveLength(100);
|
||||
expect(first.nextCursor).not.toBeNull();
|
||||
|
||||
const second = await getConvosByCursor(user, {
|
||||
limit: Number.MAX_SAFE_INTEGER,
|
||||
cursor: first.nextCursor,
|
||||
});
|
||||
|
||||
expect(second.conversations).toHaveLength(TOTAL - 100);
|
||||
expect(second.nextCursor).toBeNull();
|
||||
|
||||
const ids = new Set(
|
||||
[...first.conversations, ...second.conversations].map((convo) => convo.conversationId),
|
||||
);
|
||||
expect(ids.size).toBe(TOTAL);
|
||||
});
|
||||
|
||||
it('serves a page size inside the range unchanged', async () => {
|
||||
const result = await getConvosByCursor(user, { limit: 10 });
|
||||
|
||||
expect(result.conversations).toHaveLength(10);
|
||||
expect(result.nextCursor).not.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('subagent thread leases', () => {
|
||||
it('reserves child lineage once without overwriting a concurrent winner', async () => {
|
||||
await Conversation.init();
|
||||
|
|
|
|||
|
|
@ -54,6 +54,9 @@ const AGENT_EVENT_ACTOR_RECEIPT_RETENTION_MS = 90 * 24 * 60 * 60_000;
|
|||
const MAX_AGENT_EVENT_ACTOR_SUSPENSION_BYTES = 64 * 1_024;
|
||||
/** MeiliSearch's default `pagination.maxTotalHits` ceiling. */
|
||||
const MEILI_SEARCH_LIMIT = 1000;
|
||||
/** Ceiling for a single conversation page; the sidebar's largest request is 100. */
|
||||
const MAX_CONVO_PAGE_SIZE = 100;
|
||||
const DEFAULT_CONVO_PAGE_SIZE = 25;
|
||||
const escapeMeiliFilterValue = (value: string): string =>
|
||||
value.replace(/\\/g, '\\\\').replace(/"/g, '\\"');
|
||||
|
||||
|
|
@ -2596,7 +2599,7 @@ export function createConversationMethods(
|
|||
user: string,
|
||||
{
|
||||
cursor,
|
||||
limit = 25,
|
||||
limit = DEFAULT_CONVO_PAGE_SIZE,
|
||||
isArchived = false,
|
||||
pinned = false,
|
||||
tags,
|
||||
|
|
@ -2618,6 +2621,11 @@ export function createConversationMethods(
|
|||
) {
|
||||
const Conversation = mongoose.models.Conversation as Model<IConversation> &
|
||||
Pick<SchemaWithMeiliMethods, 'meiliSearch'>;
|
||||
/* `.limit(0)` is "no limit" to MongoDB and a negative one caps to a single batch, so an
|
||||
unclamped page size hands the caller the whole collection rather than a page of it. */
|
||||
const pageSize = Number.isFinite(limit)
|
||||
? Math.min(Math.max(Math.trunc(limit), 1), MAX_CONVO_PAGE_SIZE)
|
||||
: DEFAULT_CONVO_PAGE_SIZE;
|
||||
const filters: FilterQuery<IConversation>[] = [{ user } as FilterQuery<IConversation>];
|
||||
if (isArchived) {
|
||||
filters.push({ isArchived: true } as FilterQuery<IConversation>);
|
||||
|
|
@ -2792,11 +2800,11 @@ export function createConversationMethods(
|
|||
'conversationId endpoint title createdAt updatedAt archivedAt user model agent_id assistant_id spec iconURL chatProjectId pinned',
|
||||
)
|
||||
.sort(sortObj)
|
||||
.limit(limit + 1)
|
||||
.limit(pageSize + 1)
|
||||
.lean<IConversation[]>();
|
||||
|
||||
let nextCursor: string | null = null;
|
||||
if (convos.length > limit) {
|
||||
if (convos.length > pageSize) {
|
||||
convos.pop();
|
||||
const lastReturned = convos[convos.length - 1];
|
||||
const sortValues: Record<string, string | Date | null | undefined> = {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue