Skip to content

types: match Header to what headers.decode() actually returns - #181

Open
tada5hi wants to merge 1 commit into
mafintosh:masterfrom
tada5hi:types-match-decoded-header
Open

tada5hi wants to merge 1 commit into
mafintosh:masterfrom
tada5hi:types-match-decoded-header

Conversation

@tada5hi

@tada5hi tada5hi commented Sep 1, 2026

Copy link
Copy Markdown

Three fields in the Header interface added by #179 disagree with the object headers.decode() builds. All three are verifiable from headers.js alone.

type omits four values and cannot be null

type: 'file' | 'link' | 'symlink' | 'directory'
    | 'block-device' | 'character-device' | 'fifo' | 'contiguous-file'

toType() (headers.js:186-201) returns four more — 'pax-header' (72), 'pax-global-header' (55), 'gnu-long-link-path' (27), 'gnu-long-path' (28, 30) — and falls through to return null for any unrecognised typeflag. decode() assigns the result unguarded at :147.

Consequences today: entry.type === 'pax-header' is a compile error (TS2367, "this comparison appears to be unintentional"), and a null guard on type is flagged as statically dead when it is the only thing standing between a caller and an unrecognised entry.

linkname cannot be null

linkname: string

headers.js:104 — const linkname = buf[157] === 0 ? null : decodeStr(...). Every entry without a link target has linkname === null, which is the common case.

byteOffset is missing entirely

decode() always sets it (headers.js:140), and extract.js:153 updates it per entry (this._header.byteOffset = this._buffer.shifted). It is absent from the declaration, so reading it is a compile error on a property that always exists.


Types-only, no runtime change. Widening type and linkname also widens HeaderArgument for pack.entry(), which is consistent with the runtime: toTypeflag() (headers.js:203-224) already returns 0 for anything it does not recognise, null included.

Context

Noticed downstream. 3.2.1's bundled declarations shadow @types/tar-stream, which had modelled all 13 type values plus null, and linkname as string | null — so for these fields the DefinitelyTyped package was the more accurate of the two, and adopting the bundled types is currently a narrowing. This PR closes that gap so the bundled declarations are strictly the better source.

Three fields in the Header declaration disagree with the object decode()
builds (headers.js:139-155):

- type omits four values toType() can return — pax-header,
  pax-global-header, gnu-long-link-path, gnu-long-path — and is declared
  non-nullable although toType() returns null for any unrecognised
  typeflag (headers.js:186-201).
- linkname is declared string, but decode() assigns null when byte 157 is
  zero (headers.js:104).
- byteOffset is absent from the declaration, although decode() always sets
  it (headers.js:140) and extract.js:153 updates it per entry.
Copilot AI lite review requested due to automatic review settings September 1, 2026 12:10

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants