fix(page): move-position — валидация charset вместо длины

MovePageDto.position — это fractional-indexing ключ (generateJitteredKeyBetween,
тот же генератор, что в page.service). @MinLength(5)/@MaxLength(12) не совпадали
с реальным диапазоном генератора: плотные between-вставки в глубоком дереве
растят ключ далеко за 12 символов (замерено >40), и валидный ключ, который
сервер сам сгенерил, отклонялся 400 (Gitea #139, п.6).

Теперь валидируем по charset — base-62 алфавит [0-9A-Za-z] — плюс щедрый
@MaxLength(256) как чистый DoS-guard, сильно выше любого реального ключа.
test.failing (bug-lock) распинен в обычный it; добавлен кейс на отклонение
символов вне алфавита (control/separator/инъекция/пустая строка).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-07-11 18:49:13 +03:00
parent 73dcc4553d
commit beb61e77bb
2 changed files with 43 additions and 26 deletions
@@ -17,9 +17,10 @@ import { MovePageDto } from './move-page.dto';
// a valid ordering key the server itself generated would be refused on move.
//
// The tests below assert the CORRECT contract: any key the generator can produce
// must satisfy the DTO. The genuinely-failing case is marked `test.failing` so the
// suite stays green while locking the bug; it flips red (alerting us) once the DTO
// bounds are widened to cover the generator's real range.
// must satisfy the DTO. FIXED (#495 item 9): the DTO now validates `position` by
// CHARSET ([0-9A-Za-z], the generator's base-62 alphabet) instead of the wrong
// @MaxLength(12) length bound, so dense between-inserts are accepted; the former
// `test.failing` bug-lock is now a passing assertion.
function constraintErrors(position: unknown) {
const dto = plainToInstance(MovePageDto, {
@@ -47,24 +48,30 @@ describe('MovePageDto.position vs generateJitteredKeyBetween parity', () => {
expect(hasError(errors, 'position')).toBe(false);
});
// BUG LOCK: dense between-inserts produce keys longer than 12 chars, which
// MaxLength(12) rejects even though they are valid ordering keys. This SHOULD
// pass; it currently fails. Flips green when the DTO bound is fixed.
test.failing(
'accepts dense between-inserted keys (currently rejected by MaxLength(12))',
async () => {
let lo = generateJitteredKeyBetween(null, null);
let hi = generateJitteredKeyBetween(lo, null);
// Repeatedly insert just above `lo`, shrinking the gap so the key grows.
let longest = lo;
for (let i = 0; i < 40; i++) {
const mid = generateJitteredKeyBetween(lo, hi);
if (mid.length > longest.length) longest = mid;
hi = mid;
}
expect(longest.length).toBeGreaterThan(12); // sanity: we produced a long key
const errors = await constraintErrors(longest);
expect(hasError(errors, 'position')).toBe(false);
},
);
// FIXED: dense between-inserts produce keys longer than 12 chars, which the old
// MaxLength(12) rejected even though they are valid ordering keys. Now accepted.
it('accepts dense between-inserted keys longer than 12 chars', async () => {
let lo = generateJitteredKeyBetween(null, null);
let hi = generateJitteredKeyBetween(lo, null);
// Repeatedly insert just above `lo`, shrinking the gap so the key grows.
let longest = lo;
for (let i = 0; i < 40; i++) {
const mid = generateJitteredKeyBetween(lo, hi);
if (mid.length > longest.length) longest = mid;
hi = mid;
}
expect(longest.length).toBeGreaterThan(12); // sanity: we produced a long key
const errors = await constraintErrors(longest);
expect(hasError(errors, 'position')).toBe(false);
});
// The charset guard replaces the length bound: reject anything outside the
// generator's [0-9A-Za-z] alphabet (control chars, separators, injection) and
// the empty string, while still accepting every real key.
it('rejects a position with characters outside the fractional-index alphabet', async () => {
for (const bad of ['a0/b', 'a b', 'a\n0', 'a.b', '', "a';--"]) {
const errors = await constraintErrors(bad);
expect(hasError(errors, 'position')).toBe(true);
}
});
});
+13 -3
View File
@@ -1,8 +1,8 @@
import {
IsString,
IsOptional,
MinLength,
MaxLength,
Matches,
IsNotEmpty,
} from 'class-validator';
@@ -10,9 +10,19 @@ export class MovePageDto {
@IsString()
pageId: string;
// `position` is a fractional-indexing key from `generateJitteredKeyBetween`
// (the SAME generator page.service uses). Validate by CHARSET, not length: the
// generator's default base-62 alphabet is [0-9A-Za-z], and DENSE between-inserts
// legitimately grow a key well past a dozen chars (measured >40), so the old
// @MinLength(5)/@MaxLength(12) bounds wrongly 400'd valid ordering keys the
// server itself produced (Gitea #139 item 6). The charset regex rejects control
// chars / separators / injection, and a generous MaxLength stays only as a
// DoS guard — far above any realistic key, so it never rejects a real move.
@IsString()
@MinLength(5)
@MaxLength(12)
@Matches(/^[0-9A-Za-z]+$/, {
message: 'position must be a fractional-index key ([0-9A-Za-z])',
})
@MaxLength(256)
position: string;
@IsOptional()