fix(Label_4A_DIS): guard against comma-less DIS payloads (#493) - #520
fix(Label_4A_DIS): guard against comma-less DIS payloads (#493)#520fuleinist wants to merge 2 commits into
Conversation
…amesio#493) Label_4A_DIS.decode split message.text on ',' then unconditionally called fields[1].substring(2). A comma-less DIS payload (truncated header, DIS-only marker) made fields[1] undefined, throwing TypeError and aborting MessageDecoder.decode(). Fix: early-return with decoded=false when fields.length < 2 or fields[1] is too short for the substring(2) + '00' timestamp parse. Adds 2 regression tests for comma-less and empty-field cases.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe DIS decoder validates required fields before processing input. Malformed payloads return unsuccessful decode results without throwing. Tests cover comma-less, empty-field, and incomplete payloads. ChangesDIS payload validation
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/plugins/Label_4A_DIS.test.ts (1)
42-60: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd regression tests for the incomplete payloads in the objective.
These tests cover
DISandDIS,, but they do not coverDIS01orDIS01,190009. The latter currently passes the decoder guard and can produce a successful result with no callsign. Add both cases and assertdecodedisfalse,decodeLevelisnone, and the formatted item list is empty.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/plugins/Label_4A_DIS.test.ts` around lines 42 - 60, Add regression tests in the existing Label_4A_DIS test suite for payloads `DIS01` and `DIS01,190009`. For each case, call `plugin.decode(message)` and assert `decoded` is false, `decodeResult.decoder.decodeLevel` is `none`, and `decodeResult.formatted.items` is empty, matching the existing incomplete-payload tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/plugins/Label_4A_DIS.ts`:
- Around line 20-26: Update the validation preceding decoded assignment in the
Label_4A_DIS decoder to require at least three fields, a non-empty fields[2]
callsign, and a timestamp in fields[1] with at least six characters; keep
decoded false and return the decode result when any requirement fails. Add
regression tests covering missing callsigns and truncated timestamps.
---
Outside diff comments:
In `@lib/plugins/Label_4A_DIS.test.ts`:
- Around line 42-60: Add regression tests in the existing Label_4A_DIS test
suite for payloads `DIS01` and `DIS01,190009`. For each case, call
`plugin.decode(message)` and assert `decoded` is false,
`decodeResult.decoder.decodeLevel` is `none`, and `decodeResult.formatted.items`
is empty, matching the existing incomplete-payload tests.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a02004c2-a81b-41e0-a445-822c8b7546d7
📒 Files selected for processing (2)
lib/plugins/Label_4A_DIS.test.tslib/plugins/Label_4A_DIS.ts
| if (fields.length < 2 || !fields[1] || fields[1].length < 3) { | ||
| decodeResult.decoded = false; | ||
| this.setDecodeLevel(decodeResult, decodeResult.decoded); | ||
| return decodeResult; | ||
| } | ||
|
|
||
| decodeResult.decoded = true; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target file ---'
wc -l lib/plugins/Label_4A_DIS.ts
cat -n lib/plugins/Label_4A_DIS.ts
printf '%s\n' '--- related symbols ---'
rg -n --glob '!node_modules' \
'convertHHMMSSToTod|Label_4A_DIS|DIS01|callsign|fields\[2\]' \
. | head -n 240
printf '%s\n' '--- candidate test files ---'
git ls-files | rg -i '(^|/)(test|tests|spec|__tests__)(/|$)|Label_4A_DIS|DIS'Repository: airframesio/acars-decoder-typescript
Length of output: 13491
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- timestamp utility ---'
cat -n lib/DateTimeUtils.ts
printf '%s\n' '--- timestamp tests ---'
cat -n lib/DateTimeUtils.test.ts
printf '%s\n' '--- formatter callsign ---'
sed -n '1,125p' lib/utils/result_formatter.ts
printf '%s\n' '--- formatter tests ---'
sed -n '1,130p' lib/utils/result_formatter.test.ts
printf '%s\n' '--- plugin base initialization ---'
rg -n -A45 -B10 'initResult|setDecodeLevel' lib/DecoderPlugin.ts lib/DecoderPluginInterface.ts
printf '%s\n' '--- DIS tests ---'
cat -n lib/plugins/Label_4A_DIS.test.tsRepository: airframesio/acars-decoder-typescript
Length of output: 19553
🏁 Script executed:
#!/bin/bash
set -eu
node - <<'JS'
function convertHHMMSSToTod(time) {
if (time.length === 4) time += '00';
const h = Number(time.substring(0, 2));
const m = Number(time.substring(2, 4));
const s = Number(time.substring(4, 6));
return h * 3600 + m * 60 + s;
}
const fields = ['DIS01', '190009', 'WEN3140'];
for (const timestamp of ['190', '1900', '19000', '190009']) {
const converted = timestamp.substring(2) + '00';
console.log(JSON.stringify({
timestamp,
converted,
tod: convertHHMMSSToTod(converted),
acceptedByCurrentGuard: timestamp.length >= 3,
acceptedByRequiredGuard: timestamp.length >= 6,
}));
}
for (const text of ['DIS01,190009', 'DIS01,190009,', 'DIS01,190009,WEN3140']) {
const parts = text.split(',');
console.log(JSON.stringify({
text,
fieldCount: parts.length,
currentSuccessPath: parts.length >= 2 && !!parts[1] && parts[1].length >= 3,
completeRecord: parts.length >= 3 && !!parts[1] && !!parts[2] && parts[1].length >= 6,
callsign: parts[2],
}));
}
JSRepository: airframesio/acars-decoder-typescript
Length of output: 926
Require a complete DIS record and a six-character timestamp.
Require fields.length >= 3, a non-empty fields[2], and fields[1].length >= 6 before setting decoded to true. Add regression tests for missing callsigns and truncated timestamp fields.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/plugins/Label_4A_DIS.ts` around lines 20 - 26, Update the validation
preceding decoded assignment in the Label_4A_DIS decoder to require at least
three fields, a non-empty fields[2] callsign, and a timestamp in fields[1] with
at least six characters; keep decoded false and return the decode result when
any requirement fails. Add regression tests covering missing callsigns and
truncated timestamps.
makrsmark
left a comment
makrsmark
left a comment
There was a problem hiding this comment.
approving with non-blocking request
|
|
||
| decodeResult.decoded = true; | ||
| const fields = message.text.split(','); | ||
| if (fields.length < 2 || !fields[1] || fields[1].length < 3) { |
There was a problem hiding this comment.
pretty sure !fields[1] is redundant with fields.length
|
Addressed the review suggestion in 60f8cff: the guard now requires the callsign field ( ields[2]) and at least three comma-separated fields, so payloads like DIS01,190009 no longer decode successfully without a callsign. Added regression tests for both DIS01 (already guarded, now covered by a test) and DIS01,190009. Full suite: 485 passed / 8 skipped. Thanks for the review! |
2 participants
Problem
Label_4A_DIS.decodesplitsmessage.texton,then unconditionally callsfields[1].substring(2). A comma-less DIS payload (truncated header, DIS-only marker) makesfields[1]undefined, throwingTypeError: Cannot read properties of undefined (reading 'substring')and aborting the entireMessageDecoder.decode().Fix
Early-return with
decoded: falsewhenfields.length < 2orfields[1]is too short for the timestamp parse. This mirrors the defensive pattern used in other label plugins (e.g.Label_4T_ETA,Label_5Z_Slash).Tests
DIS01,190009,WEN3140,@HOLD CNX) still passes"DIS"(no comma) →decoded: false, no throw"DIS,"(empty field after comma) →decoded: false, no throwCloses #493
Summary by CodeRabbit
DISmessages.DISmessages continue to decode with their existing timestamps, callsigns, and text formatting.