Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 16 additions & 4 deletions packages/diffs/src/editor/textDocument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,16 +316,28 @@ export class TextDocument<LAnnotation> {
}

normalizePosition(position: Position): Position {
const line = Math.max(0, Math.min(position.line, this.lineCount - 1));
const line = TextDocument.#clampIndex(position.line, this.lineCount - 1);
return {
line,
character: Math.max(
0,
Math.min(position.character, this.getLineLength(line))
character: TextDocument.#clampIndex(
position.character,
this.getLineLength(line)
),
};
}

// Math.min/max pass NaN through, and a fractional index breaks the
// integer-keyed line-offset lookup — either way the resolved offset becomes
// NaN and a degenerate edit range can swallow the whole document. Malformed
// components clamp like any other out-of-range value: NaN and -Infinity act
// as 0, fractions floor, +Infinity clamps to the max.
static #clampIndex(value: number, max: number): number {
if (Number.isNaN(value)) {
return 0;
}
return Math.max(0, Math.min(Math.floor(value), max));
}

#resolveEdit(edit: TextEdit): ResolvedTextEdit {
let start = this.offsetAt(edit.range.start);
let end = this.offsetAt(edit.range.end);
Expand Down
8 changes: 1 addition & 7 deletions packages/diffs/test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ order. If you touch one of these areas, consider adding the missing coverage:
- **IME composition deferrals** — IME/composition interaction with editing and
undo-coalescing is deferred and not covered.

## Known bugs pinned as `test.failing` (32)
## Known bugs pinned as `test.failing` (30)

- **EditStack coalescing** (3) — coalescing decisions compare a new edit against
whatever sits on top of the undo stack purely by geometry, with no state reset
Expand Down Expand Up @@ -168,9 +168,3 @@ order. If you touch one of these areas, consider adding the missing coverage:
(or up across a logical-line boundary) can park the caret between the halves
of an astral character; a subsequent insert there splits the pair into lone
surrogates.
- **Malformed position components destroy the document** (2) — position
normalization has no finiteness/integer guard, so a `NaN` (or fractional)
line/character flows through min/max clamping into the offset computation, the
resolved offset becomes `NaN`, and the degenerate range resolves to a
whole-document replace — an insert with one malformed position component
silently erases everything else.
74 changes: 31 additions & 43 deletions packages/diffs/test/editorTextDocument.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1573,54 +1573,42 @@ function insertAtMalformed(
}

describe('malformed numeric position components', () => {
// A stricter contract would reject non-numeric components outright;
// pierre-fe's normalizePosition has no finiteness guard, so NaN flows
// through Math.min/max into offsetAt, the resolved offset becomes NaN,
// and the edit range degenerates into a whole-document replace.

test.failing(
'an insert with a NaN component never destroys unrelated content',
() => {
// KNOWN BUG: NaN in either component resolves to a NaN offset and the
// edit replaces the ENTIRE document with the inserted text.
for (const [line, character] of [
[Number.NaN, 1],
[0, Number.NaN],
[Number.NaN, Number.NaN],
]) {
const { threw, text } = insertAtMalformed(
'harbor\nlantern',
line,
character
);
if (!threw) {
expect(text).toContain('harbor');
expect(text).toContain('lantern');
}
}
// normalizePosition sanitizes malformed components the same way it clamps
// out-of-range ones: NaN and -Infinity act as 0, fractions floor, and
// +Infinity clamps to the document/line end. An edit carrying a malformed
// position therefore lands at a valid clamped spot instead of resolving to
// a NaN offset (which used to degenerate into a whole-document replace).

test('an insert with a NaN component never destroys unrelated content', () => {
// A NaN component sanitizes to 0, so the insert lands at the start of
// the resolved axis: [NaN, 1] keeps the intact character 1, [0, NaN] and
// [NaN, NaN] land at the document start.
for (const [line, character, expected] of [
[Number.NaN, 1, 'h#arbor\nlantern'],
[0, Number.NaN, '#harbor\nlantern'],
[Number.NaN, Number.NaN, '#harbor\nlantern'],
] as const) {
const { threw, text } = insertAtMalformed(
'harbor\nlantern',
line,
character
);
expect(threw).toBe(false);
expect(text).toBe(expected);
}
);
});

test.failing(
'an insert with fractional components never destroys unrelated content',
() => {
// KNOWN BUG: a fractional line (0.5) makes offsetAt return NaN via the
// line-offset lookup, taking the same whole-document-replace path as
// NaN inputs.
const { threw, text } = insertAtMalformed('harbor\nlantern', 0.5, 2.5);
if (!threw) {
expect(text).toContain('harbor');
expect(text).toContain('lantern');
}
}
);
test('an insert with fractional components floors to a valid position', () => {
const { threw, text } = insertAtMalformed('harbor\nlantern', 0.5, 2.5);
expect(threw).toBe(false);
// line 0.5 floors to 0, character 2.5 floors to 2.
expect(text).toBe('ha#rbor\nlantern');
});

test('Infinity components clamp to a valid position without data loss', () => {
// DIVERGENCE: a stricter contract would reject non-finite components by
// throwing; pierre-fe's Math.min/max clamping happens to bound Infinity
// to a real offset, so the insert lands at a valid spot and nothing is
// lost. Pinned so a future finiteness guard (which may prefer to throw)
// shows up here.
// throwing; the sanitizing clamp bounds +Infinity to the last line
// instead, keeping every position-taking API total.
const { threw, text } = insertAtMalformed('harbor\nlantern', Infinity, 0);
expect(threw).toBe(false);
expect(text).toContain('harbor');
Expand Down