Skip to content

feat: add support for TABNAME and COLINFO tokens - #1770

Merged
arthurschreiber merged 4 commits into
masterfrom
claude/tabname-colinfo-tokens
Sep 1, 2026
Merged

feat: add support for TABNAME and COLINFO tokens#1770
arthurschreiber merged 4 commits into
masterfrom
claude/tabname-colinfo-tokens

Conversation

@arthurschreiber

@arthurschreiber arthurschreiber commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes #242. Fixes #410.

Problem

SQL Server sends TABNAME (0xA4) and COLINFO (0xA5) tokens for queries executed in browse mode — a FOR BROWSE clause, SET NO_BROWSETABLE ON, or the sp_cursoropen/sp_cursorfetch API cursor procedures (MS-TDS TABNAME, MS-TDS COLINFO). These tokens describe how the columns of a result set map back to their base tables.

Previously, tedious had no parsers for either token, so receiving one crashed the token parser with an uncaught Unknown type: 164 / Unknown type: 165 error, taking down the whole process. Verified against tedious 20.0.0 and SQL Server 2022:

CREATE TABLE #t ([id] int PRIMARY KEY);
INSERT INTO #t VALUES (1);
SET NO_BROWSETABLE ON;
SELECT [id] FROM #t; -- uncaught exception: Unknown type: 164

Changes

  • src/token/tabname-token-parser.ts — parses TABNAME: the list of base table names referenced by the query, each parsed into its parts (e.g. ['dbo', 'employees']). The multi-part format is used on all TDS versions: it was introduced in TDS 7.1 Revision 1, and all servers speaking TDS 7.1 or newer send it (verified against SQL Server 2022 on a TDS 7.1 connection).
  • src/token/colinfo-token-parser.ts — parses COLINFO: per-column ordinal, base table number (a one-based index into the TABNAME list), the EXPRESSION/KEY/HIDDEN status flags, and the base column name when the column is aliased (DIFFERENT_NAME).
  • Both tokens are wired into the stream parser using the standard NotEnoughDataError retry pattern, and surfaced as new documented tabName and colInfo events on Request, following the existing order event's pattern. All other token handlers treat them as unexpected tokens, same as every other token type.
  • Both parsers throw a descriptive error if a token's contents overrun its declared length, instead of silently desyncing the token stream on malformed data.

Tests

  • Unit (test/unit/token/tabname-token-parser-test.ts, test/unit/token/colinfo-token-parser-test.ts): one-part/multi-part/multiple table names, TDS 7.1 behavior, all status flag combinations, aliased columns, tokens arriving fragmented in single-byte chunks, and malformed-token rejection.
  • Integration (test/integration/browse-mode-test.ts): FOR BROWSE queries (including the server-appended hidden key columns being reported via colInfo), multi-table joins (multiple TABNAME entries and per-table column mapping), expression columns via a parameterised RPC request, API cursors via sp_cursoropen, and the exact SET NO_BROWSETABLE ON scenario from Token parser for TABNAME (0xA4) non-existent #410. Verified passing against a real SQL Server 2022 instance on TDS 7.1, 7.2, and 7.4 connections.

Full unit suite, integration suite, eslint, and tsc all pass.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Fxv5h4UMCGJEpjgCKcAxug

The server sends TABNAME (0xA4) and COLINFO (0xA5) tokens for queries
executed in browse mode (a `FOR BROWSE` clause, `SET NO_BROWSETABLE ON`,
or the `sp_cursoropen`/`sp_cursorfetch` API cursor procedures). These
tokens describe how the columns of a result set map back to their base
tables.

Previously, receiving either token crashed the token parser with an
uncaught "Unknown type" error, rendering browse mode queries unusable.
Now both tokens are parsed and surfaced via new `tabName` and `colInfo`
events on `Request`.

Fixes #242. Fixes #410.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fxv5h4UMCGJEpjgCKcAxug
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-01T11:37:30.619557Z ab63a8d New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Sep 1, 2026

Copy link
Copy Markdown

Review

Solid, well-scoped fix. It follows the existing COLMETADATA/ORDER parser conventions closely (length-prefixed bounds check before parsing, NotEnoughDataError retry pattern in stream-parser.ts, dual export default/module.exports), and the TDS version branch in tabname-token-parser.ts is a faithful copy of readTableName in colmetadata-token-parser.ts. Field parsing matches the MS-TDS spec (status flags 0x04/0x08/0x10/0x20, one-based TableNum, BVarChar base column name gated on DIFFERENT_NAME). Both new parsers correctly consume at least one byte per loop iteration, so there's no risk of an infinite loop from a malformed/malicious tokenLength.

Nice test coverage too: fragmented single-byte-chunk parsing, both TDS-version branches for TABNAME, all status flag combinations for COLINFO, and integration tests covering both the FOR BROWSE and SET NO_BROWSETABLE ON repro from #410.

Minor / worth a second look

  • onTabName/onColInfo are only overridden on RequestTokenHandler. InitialSqlTokenHandler overrides onOrder/onColMetadata/onRow to gracefully emit a connection error + close() instead of letting the base TokenHandler throw UnexpectedTokenError, even though those tokens are just as unlikely to appear in the initial-SQL token stream as TABNAME/COLINFO are. In practice browse-mode tokens should never appear outside an active Request, so this is probably fine, but it's a slight inconsistency with the defensive pattern used for the other "shouldn't happen" token types in that handler — worth confirming that an unexpected TABNAME/COLINFO arriving during login/initial-SQL/attention-ack can't still crash the process the same way the original bug did, rather than just falling through to the (now unreachable in practice) default throw.
  • ColumnInfo.colName is typed as string | undefined rather than an optional property (colName?: string). Not wrong, but colName?: string is the more idiomatic way to express "may be absent" and matches how optional fields tend to be modeled elsewhere (e.g. ColumnMetadata.tableName?).
  • The tableNum doc comment ("0 if the column is the result of an expression") is a helpful addition beyond the raw spec — nice touch for consumers who won't want to go read MS-TDS themselves.

No security or performance concerns — parsing is bounded, synchronous, and doesn't allocate anything unbounded relative to the token's declared length.

Overall this looks safe to merge; the one item above is worth a quick sanity check but isn't blocking.

Add integration tests for the browse mode scenarios legacy applications
commonly produce: multi-table joins (multiple TABNAME entries and
per-table column mapping), expression columns (including via a
parameterised RPC request), and API cursors via `sp_cursoropen`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fxv5h4UMCGJEpjgCKcAxug
- Parse TABNAME table names using the multi-part format on all TDS
  versions. The single-string format only existed before TDS 7.1
  Revision 1, and servers speaking TDS 7.1 or newer always send the
  multi-part format - the previous version check misparsed TABNAME
  tokens on TDS 7.1 connections. This also simplifies the `tabName`
  event's payload type to `string[][]`.
- Throw a descriptive error when a TABNAME or COLINFO token's contents
  overrun the token's declared length, instead of silently desyncing
  the token stream on malformed data.
- Make the browse mode integration tests independent of the negotiated
  TDS version.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fxv5h4UMCGJEpjgCKcAxug
@claude

claude Bot commented Sep 1, 2026

Copy link
Copy Markdown

Review: TABNAME/COLINFO token support

Solid piece of work overall — clean implementation that follows the codebase's existing token-parser conventions closely, well-documented public API, and unusually thorough test coverage (unit tests for fragmented/malformed input, integration tests against real SQL Server across multiple TDS versions).

Correctness

  • TYPE.TABNAME (0xA4) / TYPE.COLINFO (0xA5) and the field layouts match MS-TDS 2.2.7.23 / 2.2.7.6 (NumParts + US_VARCHAR parts for TABNAME; ColNum/TableNum/Status + optional B_VARCHAR name for COLINFO), and the status flag bit values (EXPRESSION=0x04, KEY=0x08, HIDDEN=0x10, DIFFERENT_NAME=0x20) check out.
  • Always using the multi-part TABNAME format regardless of TDS version is correct since tedious's minimum supported version is already 7.1 (src/tds-versions.ts), which is where that format was introduced — good catch documenting that reasoning in a comment.
  • The "malformed token" checks (offset !== end after the loop) are a nice defensive touch — without them, a token whose declared length doesn't match its actual contents would silently desync the rest of the token stream instead of failing loudly. Both new parsers guarantee forward progress per loop iteration (minimum 1 read consumes ≥1 byte), so there's no infinite-loop risk on malformed input.
  • NotEnoughDataError retry wiring in stream-parser.ts follows the exact same pattern as the other token readers (position is only committed after a successful parse), so partial-chunk delivery is handled correctly.
  • Unhandled-token behavior (onTabName/onColInfo throwing UnexpectedTokenError by default) is consistent with how onOrder and friends are handled in contexts where these tokens shouldn't appear.

Minor nit (non-blocking)

  • The doc comment on ColumnInfo.tableNum says it's "0 if the column is the result of an expression" — per the TDS spec, tableNum can be 0 for other reasons too (e.g. constant/computed columns that aren't flagged EXPRESSION), so the comment reads as slightly narrower than reality. Might be worth softening to something like "0 if the column isn't derived from a base table (e.g. an expression)".

Security / Performance

Tests

  • Great coverage: single/multi-part names, multiple tables, status flag combinations, aliasing, byte-by-byte fragmented delivery, and malformed-length rejection at the unit level; FOR BROWSE, joins, parameterized expression columns, API cursors (sp_cursoropen/sp_cursorfetch), and the exact SET NO_BROWSETABLE ON regression from Token parser for TABNAME (0xA4) non-existent #410 at the integration level.

Nothing here blocks merging — nice fix for a long-standing crash bug.

@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.44262% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.94%. Comparing base (3aba7c7) to head (113fd5f).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/token/handler.ts 33.33% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1770      +/-   ##
==========================================
+ Coverage   80.86%   80.94%   +0.07%     
==========================================
  Files          90       92       +2     
  Lines        4887     4948      +61     
  Branches      929      938       +9     
==========================================
+ Hits         3952     4005      +53     
- Misses        638      646       +8     
  Partials      297      297              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Use the same spec revision numbering as the other token parsers in this
directory, where COLINFO is s2.2.7.3 and TABNAME is s2.2.7.20.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fxv5h4UMCGJEpjgCKcAxug
@claude

claude Bot commented Sep 1, 2026

Copy link
Copy Markdown

Review

Nice work — this is a clean, well-scoped fix for a real crash (Unknown type: 164/165 on TABNAME/COLINFO), and it follows the existing token-parser conventions closely.

Code quality

  • tabname-token-parser.ts and colinfo-token-parser.ts mirror the established pattern (order-token-parser.ts, returnvalue-token-parser.ts): Result/NotEnoughDataError, dual export default + module.exports, MS-TDS section comment header. Wiring into stream-parser.ts and handler.ts (default throw new UnexpectedTokenError in the base TokenHandler, override only in RequestTokenHandler) is consistent with how order is handled.
  • The tabName/colInfo JSDoc on Request.on/emit matches the style of the existing order event documentation, so generated docs should stay consistent.
  • Minor/non-blocking nit: colinfo-token-parser.ts uses colNum: colNum, tableNum: tableNum instead of object-shorthand — purely cosmetic, no lint rule enforces shorthand here, so feel free to ignore.

Correctness

  • The "reject a token whose contents overrun its declared length" tests are worth calling out as solid: since the DIFFERENT_NAME string read isn't bounded by end inside the loop body, a malformed token can read past the declared boundary before the trailing offset !== end check catches it. That's fine for well-formed servers and is exactly what the tests exercise, but it's worth being aware that detection happens after the overrun read rather than preventing it — as long as the read doesn't run past the actual buffer (which would just trigger another NotEnoughDataError and wait for more data, not corrupt state), this is safe.
  • Good handling of the TDS-version question: since tedious's minimum supported version is 7.1 and the multi-part TABNAME format was introduced in 7.1 Revision 1, not branching on tdsVersion is correct — and it's explicitly tested against 7_1.
  • tableNum: 0 for expression columns and the one-based table index are consistent with MS-TDS semantics as described.

Test coverage

Thorough on both levels:

  • Unit tests cover one-part/multi-part/multiple table names, all status-flag combinations, aliased columns, single-byte-chunked delivery (exercises the NotEnoughDataError retry path), and malformed-token rejection.
  • Integration tests cover the actual regression scenarios (FOR BROWSE, SET NO_BROWSETABLE ON from Token parser for TABNAME (0xA4) non-existent #410, joins producing multiple TABNAME entries, parameterised expression columns, and sp_cursoropen/sp_cursorfetch API cursors) — this is a good breadth of real-world browse-mode usage, not just the minimal repro.

Minor observations (non-blocking)

  • ColumnInfo (like the existing ColumnMetadata) isn't re-exported from src/tedious.ts, so consumers need a deep import (tedious/lib/token/token) to name the type explicitly. That matches the existing precedent for ColumnMetadata, so it's not a regression, but since this PR is adding new public event payloads, it might be worth reconsidering for both types together in a follow-up.
  • Section references (s2.2.7.3 for COLINFO, s2.2.7.20 for TABNAME) look plausible relative to the other section comments in this directory (e.g. s2.2.7.14 for ORDER) — worth a quick double check against the MS-TDS revision you verified against, but this is cosmetic only.

Security / performance

No concerns — parsing is a single bounded pass per token, all reads are length-checked before use, and malformed data throws a descriptive error instead of desyncing the stream (a real improvement over silently misinterpreting subsequent tokens).

Overall this looks solid and ready to merge pending CI.

@arthurschreiber arthurschreiber changed the title feat: add support for TABNAME and COLINFO tokens feat: add support for TABNAME and COLINFO tokens Sep 1, 2026
@arthurschreiber
arthurschreiber merged commit 170fabc into master Sep 1, 2026
94 of 104 checks passed
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 20.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Token parser for TABNAME (0xA4) non-existent Add support for TDS Tokens (TABNAME, COLINFO)

2 participants