microsoft / microsoft/tsdoc

TokenReader subrange bounds enforced inconsistently: peekToken/peekPreviousTokenKind/backtrackToMarker ignore embedded window

Open
#481 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
5k
Forks
162
Avg merge
17h 24m
Merged PRs (30d)
8

Description

Summary

TokenReader respects its embedded subrange (_readerStartIndex/_readerEndIndex) in peekTokenKind()/peekTokenAfterKind()/peekTokenAfterAfterKind(), but three sibling methods ignore those bounds: peekToken() has no end guard at all, and peekPreviousTokenKind() / backtrackToMarker() compare against 0 / allow rewinding before the subrange start. This leaks outer tokens into embedded parses and can return undefined where Token is promised.

Location

  • File: tsdoc/src/parser/TokenReader.ts
  • Class: TokenReader
  • Methods:
    • peekToken(): Tokenreturn this.tokens[this._currentIndex]; with no _readerEndIndex check
    • peekPreviousTokenKind()if (this._currentIndex === 0) instead of === this._readerStartIndex
    • backtrackToMarker(marker) — checks marker > this._currentIndex but not marker < this._readerStartIndex

Contrast with the guarded siblings in the same file:

public peekTokenKind(): TokenKind {
  if (this._currentIndex >= this._readerEndIndex) {
    return TokenKind.EndOfInput;
  }
  ...
}

Problem

  1. peekToken() past end returns undefined: return type claims Token, but past _readerEndIndex the index expression yields undefined. Callers doing peekToken().range / peekToken().kind then throw TypeError: Cannot read properties of undefined instead of getting a clean EndOfInput signal. Current internal hot paths in NodeParser.ts happen to call peekTokenKind() first (e.g. block/inline tag badCharacter paths), which masks the hole, but the public API contract is broken for any external consumer or future internal use.
  2. peekPreviousTokenKind() leaks across embedded start: for an embedded reader starting at index N>0, calling it at the embedded start returns tokens[N-1].kind (outer context) instead of EndOfInput. NodeParser._parseBlockTag and _parseFencedCode switch on this to decide start-of-input behavior (AtSignInWord, CodeFenceOpeningIndent); an embedded @ or fence at the subrange start can therefore be misclassified using the token before the subrange.
  3. backtrackToMarker() can rewind before subrange start: nothing prevents marker < _readerStartIndex, letting a later readToken() consume outer tokens that the embedded reader was explicitly scoped to exclude.

Trigger / Reproduction

Based on static analysis (no execution performed):

  • Construct new TokenReader(parserContext, embeddedSequence) where embeddedSequence.startIndex > 0, advance to the embedded start, and call peekPreviousTokenKind() — returns the outer predecessor kind instead of EndOfInput.
  • Call peekToken() when _currentIndex === _readerEndIndex — returns undefined instead of signaling end (compare peekTokenKind() returning EndOfInput in the same state).
  • Call backtrackToMarker(outerMarker) with outerMarker < _readerStartIndex on an embedded reader — accepted, subsequent reads escape the subrange.

Note: this is a static-analysis finding; I did not execute a parser fixture.

Expected Behavior

All TokenReader accessors/mutators should honor the same [ _readerStartIndex, _readerEndIndex ) window: peekToken() should signal end (or throw a clear parser-bug error like readToken() does) instead of returning undefined; peekPreviousTokenKind() should return EndOfInput at the subrange start; backtrackToMarker() should reject markers below the subrange start.

Actual Behavior

Subrange bounds are enforced inconsistently, so embedded parses can observe outer tokens and end-of-input handling differs by method.

Impact

  • Potential spurious AtSignInWord / fence-indent errors for embedded constructs starting at a subrange boundary.
  • undefined-token TypeErrors for API consumers using peekToken() at end, bypassing the clean EndOfInput protocol every sibling method follows.

Suggested Direction

  • Mirror the existing peekTokenKind() guard in peekToken() (return the EndOfInput token or throw a parser-bug error — maintainer's choice, but document it), change the peekPreviousTokenKind() zero-check to _readerStartIndex, and add a marker < _readerStartIndex rejection in backtrackToMarker(). No grammar changes needed.

Evidence

  • Source via API: tsdoc/src/parser/TokenReader.ts shows guarded peekTokenKind/peekTokenAfterKind/peekTokenAfterAfterKind vs unguarded peekToken/peekPreviousTokenKind/backtrackToMarker; tsdoc/src/parser/NodeParser.ts shows peekPreviousTokenKind gating AtSignInWord/CodeFenceOpeningIndent and embedded TokenReader construction for scoped parses.
  • Duplicate check: issue search for peekToken TokenReader bounds returns total_count: 0, and open issues contain no TokenReader-boundary report — no apparent duplicate. This is a non-security correctness finding, so Microsoft's private-security-disclosure requirement does not apply.

Classification

  • FACT: three TokenReader methods ignore the subrange window that three sibling methods enforce (verified in source via API).
  • INFERENCE: embedded parses can observe out-of-range tokens; end-of-input peekToken() yields undefined.
  • HYPOTHESIS: aligning all methods on the same window fixes the misclassification/TypeError risk with no grammar change.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in tsdoc/src/parser/TokenReader.ts by comparing peekToken(), peekPreviousTokenKind(), and backtrackToMarker() with the guarded sibling methods. Review the embedded-reader behavior and the NodeParser call sites named in the issue. Done means all three methods honor the [_readerStartIndex, _readerEndIndex) window and the chosen end-of-input behavior is covered by tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
compilers
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.