Skip to content

Fix optional()/nullable() crashing on absent dynamic() values - #1302

Open
yhuikzdtguioaert wants to merge 1 commit into
ianstormtaylor:mainfrom
yhuikzdtguioaert:fix/optional-nullable-dynamic-crash
Open

yhuikzdtguioaert wants to merge 1 commit into
ianstormtaylor:mainfrom
yhuikzdtguioaert:fix/optional-nullable-dynamic-crash

Conversation

@yhuikzdtguioaert

Copy link
Copy Markdown

Fixes #1294.

Problem

optional() and nullable() only guard their validator and refiner against undefined/null. They copy the wrapped struct's entries and coercer through the object spread, so when the value is absent the inner struct is still traversed and coerced.

That is harmless for most structs, whose entries bail out on a non-matching value, but dynamic() runs its selector callback unconditionally. So an absent value reaches user code that assumes it is present:

import { assert, object, partial, dynamic, literal } from 'superstruct'

const User = object({ kind: literal('user') })
const Bot = object({ kind: literal('bot') })
const entity = dynamic((v) => (v.kind === 'user' ? User : Bot))

assert({}, partial(object({ entity })))
// throws: TypeError: Cannot read properties of undefined (reading 'kind')
// expected: passes, since `entity` is optional under partial()

The same happens for optional(dynamic(...)) with undefined and nullable(dynamic(...)) with null, on the create() path (via the inherited coercer) as well as assert() / is() / validate().

Fix

Guard entries and coercer in optional() and nullable() so an undefined (optional) or null (nullable) value short-circuits without descending into the inner struct, matching how validator and refiner already treat the absent value as valid.

Tests

Added validation fixtures for the reported partial(object({ ...dynamic })) case plus optional(dynamic()) and nullable(dynamic()) directly, covering both the assert and create paths. The full suite passes (npm run test:vitest, and tsc over the test project).

`optional()` and `nullable()` only guard their `validator` and `refiner`
against `undefined`/`null`; they inherit the wrapped struct's `entries`
and `coercer` through the object spread. So when the value is absent the
inner struct is still traversed and coerced.

For a `dynamic()` inner this throws, because its `entries`/`coercer`
invoke the selector callback with the absent value. For example:

    assert({}, partial(object({ user: dynamic((v) => v.kind === 'user' ? User : Bot) })))

crashes with `TypeError: Cannot read properties of undefined` instead of
accepting the missing optional property.

Guard `entries` and `coercer` so an `undefined` (optional) or `null`
(nullable) value short-circuits without touching the inner struct.
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.

Dynamic type fails when used inside "partial" helper

1 participant