fix(molang): prevent NaN diagnostic ranges from crashing the client (#588)

A degenerate molang node (e.g. an empty `{}` produced by the parser for
particle component objects) has an undefined `position`. The
Marker/default case in the molang syntax diagnoser passed that undefined
position straight to `diagnoser.add`, where `GetRange` resolved it via
`doc.positionAt(undefined)` to a `character: NaN`. That NaN serializes
to `null` over JSON-RPC, making the client's `asDiagnostics` reject the
entire diagnostic batch with 'Invalid arguments' and drop all
diagnostics for the file.

Fixes, in defense-in-depth layers:
- Guard the degenerate node's position with a finite fallback in the
molang syntax diagnoser.
- Clamp non-finite/out-of-bounds offsets to the document bounds in
GetRange and GetPosition so no caller can produce a NaN range.

Adds regression tests for both layers.

Co-authored-by: Claude <noreply@anthropic.com>

Author: DaanV2

ide/base/server/src/util/document-location.test.ts

@@ -1,6 +1,8 @@
-import { GetRange } from './document-location';
+import { GetRange, GetPosition } from './document-location';
 import * as vstd from 'vscode-languageserver-textdocument';
 
+const isFinitePosition = (p: vstd.Position) => Number.isFinite(p.line) && Number.isFinite(p.character);
+
 describe('GetRange', () => {
   test('returns a valid zero-length range at end of document', () => {
     const doc = vstd.TextDocument.create('file:///test.json', 'json', 1, '{\n}');
@@ -11,4 +13,38 @@ describe('GetRange', () => {
     expect(range.start).toEqual(doc.positionAt(offsetAtEof));
     expect(range.end).toEqual(doc.positionAt(offsetAtEof));
   });
+
+  // Regression: a degenerate molang node (e.g. empty `{}`) has an undefined position.
+  // Without clamping this produced a NaN character that serializes to `null` over
+  // JSON-RPC, causing the client to drop the entire diagnostic batch.
+  test.each([undefined, NaN, Infinity, -Infinity, -5])(
+    'produces a finite range for non-finite/out-of-bounds offset %p',
+    (offset) => {
+      const doc = vstd.TextDocument.create('file:///test.json', 'json', 1, '{\n}');
+
+      const range = GetRange(offset as unknown as number, doc);
+
+      expect(isFinitePosition(range.start)).toBe(true);
+      expect(isFinitePosition(range.end)).toBe(true);
+    },
+  );
+
+  test('produces a finite range for an OffsetWord with a non-finite offset', () => {
+    const doc = vstd.TextDocument.create('file:///test.json', 'json', 1, '{\n}');
+
+    const range = GetRange({ offset: NaN, text: '{}' } as any, doc);
+
+    expect(isFinitePosition(range.start)).toBe(true);
+    expect(isFinitePosition(range.end)).toBe(true);
+  });
+});
+
+describe('GetPosition', () => {
+  test.each([undefined, NaN, Infinity, -5])('produces a finite position for offset %p', (offset) => {
+    const doc = vstd.TextDocument.create('file:///test.json', 'json', 1, '{\n}');
+
+    const position = GetPosition(offset as unknown as number, doc);
+
+    expect(isFinitePosition(position)).toBe(true);
+  });
 });

ide/base/server/src/util/document-location.ts

@@ -3,6 +3,24 @@ import { Range } from 'vscode-languageserver';
 import * as vstd from 'vscode-languageserver-textdocument';
 import { Character } from './character';
 
+/**
+ * Clamps an offset into the valid `[0, text.length]` range for the given document.
+ *
+ * A non-finite offset (`undefined`, `NaN`, `Infinity`) would otherwise flow into
+ * `doc.positionAt(...)` and produce a `character: NaN`, which serializes to `null`
+ * over JSON-RPC and makes the client reject the entire diagnostic batch.
+ * @param offset The raw offset to clamp
+ * @param doc The document the offset refers to
+ * @returns A finite offset within the document's bounds
+ */
+function clampOffset(offset: number, doc: vstd.TextDocument): number {
+  if (!Number.isFinite(offset)) return 0;
+  const max = doc.getText().length;
+  if (offset < 0) return 0;
+  if (offset > max) return max;
+  return offset;
+}
+
 /**
  *
  * @param position
@@ -23,11 +41,14 @@ export function GetRange(position: DocumentLocation, doc: vstd.TextDocument): Ra
     position = doc.offsetAt(position);
     //If document location is already an offset, then grab the start position
   } else if (OffsetWord.is(position)) {
-    Start = doc.positionAt(position.offset);
-    End = doc.positionAt(position.text.length + position.offset);
+    const offset = clampOffset(position.offset, doc);
+    const length = Number.isFinite(position.text.length) ? Math.max(0, position.text.length) : 0;
+    Start = doc.positionAt(offset);
+    End = doc.positionAt(clampOffset(offset + length, doc));
 
     return { start: Start, end: End };
   } else {
+    position = clampOffset(position, doc);
     Start = doc.positionAt(position);
   }
 
@@ -64,9 +85,9 @@ export function GetRange(position: DocumentLocation, doc: vstd.TextDocument): Ra
 export function GetPosition(position: DocumentLocation, doc: vstd.TextDocument): vstd.Position {
   if (Position.is(position)) return position;
   if (JsonPath.is(position)) return resolveJsonPath(position, doc).start;
-  if (OffsetWord.is(position)) return doc.positionAt(position.offset);
+  if (OffsetWord.is(position)) return doc.positionAt(clampOffset(position.offset, doc));
 
-  return doc.positionAt(position);
+  return doc.positionAt(clampOffset(position, doc));
 }
 
 /**Resolves a json path to a range

packages/bedrock-diagnoser/src/diagnostics/molang/expressions.ts

@@ -173,8 +173,12 @@ export function diagnose_molang_syntax(expression: ExpressionNode, diagnoser: Di
 
       case NodeType.Marker:
       default:
+        // A degenerate node (e.g. an empty `{}` produced by the parser) can have an
+        // undefined position. Falling through to diagnoser.add with `undefined` makes
+        // the range resolve to a NaN character, which serializes to `null` over
+        // JSON-RPC and causes the client to drop the whole diagnostic batch.
         diagnoser.add(
-          n.position,
+          n.position ?? 0,
           `unknown piece of molang syntax: ${JSON.stringify(n)}`,
           DiagnosticSeverity.error,
           'molang.diagnoser.syntax',

packages/bedrock-diagnoser/test/lib/diagnostics/molang/degenerate-node.test.ts

@@ -0,0 +1,21 @@
+import { diagnose_molang_syntax } from '../../../../src/diagnostics/molang';
+import { TestDiagnoser } from '../../../diagnoser';
+
+describe('Molang degenerate node', () => {
+  // Regression: a degenerate node (e.g. an empty `{}` produced by the parser) has no
+  // `position`. Reporting it with `position === undefined` made the downstream range
+  // resolve to a NaN character, which serializes to `null` over JSON-RPC and caused the
+  // client to drop the entire diagnostic batch.
+  test('reports an unknown-syntax diagnostic with a finite position for an empty `{}` node', () => {
+    const diagnoser = new TestDiagnoser();
+
+    // An empty object stands in for the degenerate node observed in the wild.
+    diagnose_molang_syntax({} as any, diagnoser);
+
+    expect(diagnoser.items).toHaveLength(1);
+    const item = diagnoser.items[0];
+    expect(item.code).toBe('molang.diagnoser.syntax');
+    expect(typeof item.position).toBe('number');
+    expect(Number.isFinite(item.position as number)).toBe(true);
+  });
+});