Skip to content

fix: guard nullable 'message' fields in v2 ruleset (issue #863) - #1229

Open
tzh476 wants to merge 5 commits into
asyncapi:masterfrom
tzh476:fix/863-nullable-message-payload
Open

fix: guard nullable 'message' fields in v2 ruleset (issue #863)#1229
tzh476 wants to merge 5 commits into
asyncapi:masterfrom
tzh476:fix/863-nullable-message-payload

Conversation

@tzh476

@tzh476 tzh476 commented Aug 23, 2026

Copy link
Copy Markdown

Fixes #863.

When a payload object has a nullable property literally named message, v2-ruleset selectors navigating @property === "message" read .schemaFormat/.oneOf on null and crash validation.

Guard added: !@null && before those reads in ruleset/v2/ruleset.ts (message-examples, payload default/examples, operation-message) and ruleset/v2/functions/messageExamples-spectral-rule-v2.ts.

Verified: the issue example now validates with no error diagnostic; ruleset/validate suite passes (43 suites / 244 tests).

…syncapi#863)

Change-Id: Ia9549f33f1c06d3a56c4af9f6d01c6596e645a98
@changeset-bot

changeset-bot Bot commented Aug 23, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ff13cd3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@asyncapi/parser Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Change-Id: I9fa2cccd89ac357e05c1be34432b042d55edb15c
@tzh476

tzh476 commented Aug 24, 2026

Copy link
Copy Markdown
Author

Hi maintainers - friendly ping: this PR (fixes #863, guard nullable message fields) is ready and MERGEABLE, changeset added, CI/SonarCloud passing; happy to adjust on review. Thank you.

tzh476 added 2 commits August 29, 2026 01:11
…ge guards

This PR changed eight JSONPath expressions but shipped no test, so nothing
prevented the guards from being dropped again.

The test drives the public `parser.validate()` path with the exact document
shape from issue asyncapi#863 -- a schema property literally named `message` whose
example value is `null`.

Removing the `!@null` guards makes it fail with the error from the issue:

    TypeError: Cannot read properties of null (reading 'schemaFormat')

One detail worth recording, because it decides what the test must assert:
`validate()` **resolves** even on the unfixed code and reports the crash as a
diagnostic rather than rejecting. A "does it throw" assertion therefore passes
both before and after the fix and would have guarded nothing. The two
assertions that actually catch the regression are the diagnostic check and the
`parse()` check.

Measured on this branch:

    with guards      3 passed
    guards removed   2 failed, 1 passed   (the throw-based one is the passer)

Change-Id: I7b412b6d5b11132d332913a49ca0263f7c578bef
Disclosure: prepared by a human working with an LLM assistant.
@tzh476

tzh476 commented Aug 29, 2026

Copy link
Copy Markdown
Author

Pushed 7cead1a. This PR changed eight JSONPath expressions but shipped no test, so nothing prevented the guards from being dropped again. That is now fixed.

The new spec drives the public parser.validate() path with the exact document shape from #863 — a schema property literally named message whose example value is null.

Control experiment, guards removed:

TypeError: Cannot read properties of null (reading 'schemaFormat')
→ 2 failed, 1 passed

Guards in place: 3 passed.

One detail worth recording, because it decides what the test has to assert: validate() resolves even on the unfixed code and reports the crash as a diagnostic rather than rejecting. So a "does it throw" assertion passes both before and after the fix and would have guarded nothing — which is exactly the trap I would have fallen into. The two assertions that actually catch the regression are the diagnostic check (/Cannot read properties of null/ must be absent) and the parse() check.

Current state: mergeable: true, no conflicts, SonarCloud quality gate passed.

For context on the fix itself: @property === 'message' also matches a schema property named message, not only a message object, so @.schemaFormat dereferences null. Adding !@null skips null values before the member access — the same guard already present on the sibling oneOf and components.messages expressions in those same arrays, so this makes the set internally consistent rather than introducing a new pattern.

Disclosure: human + LLM collaboration; the numbers above are from runs on this branch.

@tzh476

tzh476 commented Sep 1, 2026

Copy link
Copy Markdown
Author

Verified this fix at the JSONPath-engine level, including a control for the thing a null-guard could
plausibly break. Posting because the PR previously rested on reasoning plus CI rather than a local
run.

I could not complete npm install for the repo on this machine (npm ends in Exit handler never called; from a container, registry.npmjs.org is unreachable entirely). Rather than leave it
unverified, I installed just nimma@0.2.2 — the JSONPath engine Spectral runs these given
expressions through — and drove the two expressions from this diff directly.

Both arms, on the document shape from #863 (a schema property literally named message whose
example value is null):

BEFORE  THREW   TypeError: Cannot read properties of null (reading 'schemaFormat')
AFTER   OK      hits=1  ["channels.user/signup.publish.message"]

The thrown message reproduces the error in the issue verbatim, and — the part that matters — the
guarded expression still matches the genuine message object. So !@null removes the crash without
dropping the match the rule exists to make.

Control, since a filter that fixes a crash can also silently stop matching things:

document before after
two normal messages [publish.message, subscribe.message] same IDENTICAL
message with schemaFormat set [] [] IDENTICAL

Behaviour is unchanged on valid documents; only the null case differs.

What this does not cover, stated plainly: this exercises the fix, not the repo's own spec. I have
not run packages/parser/test/nullable-message-863.spec.ts locally — that still needs a working
npm install, which this host cannot currently do. The CI on this PR is green (SonarCloud quality gate
passed), but per the usual caveat that is a scanner, not proof the new spec ran.

The four remaining given expressions in the diff already had !@null; this PR adds it to the four
that did not, which is why they are the ones that crash.

@sonarqubecloud

sonarqubecloud Bot commented Sep 3, 2026

Copy link
Copy Markdown

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.

Payload property named message breaks with null value

1 participant