Additional Label 80 parsing #304
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughLabel_80 parser was rewritten to support CSV and header/tag input paths with new private helpers (parseHeader, parseTags, parseCsvFormat). Tests were restructured to use a shared plugin instance and expanded raw-field assertions. ResultFormatter gained airspeed and mach formatters. Changes
Sequence Diagram(s)sequenceDiagram
participant Input as ACARS Message
participant L80 as Label_80 Plugin
participant Header as parseHeader()
participant Tags as parseTags()
participant CSV as parseCsvFormat()
participant RF as ResultFormatter
participant Output as DecodeResult
Input->>L80: Receive raw message (one+ lines)
L80->>L80: Check for CSV-style (single line with comma)
alt CSV Format
L80->>CSV: parseCsvFormat(line)
CSV->>RF: emit position, time, temp, airspeed, fuel
RF->>Output: set raw fields & formatted items
else Header/Tags Format
L80->>Header: parseHeader(first line)
Header->>RF: populate metadata (tail, flight, times)
RF->>Output: update raw/format
L80->>Tags: parseTags(remaining lines)
Tags->>RF: for each tag (POS, ALT, MCH, TAS/SPD, FOB, ETA, HDG, NWYP, SWN, DWN, AD) emit raw fields & formatted items
Tags->>Tags: assemble wind if dir+spd present
RF->>Output: update raw/format
end
L80->>Output: set decode level/decoded flag
Output-->>Input: return DecodeResult
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Pull request overview
This PR enhances the Label_80 decoder to support additional message format variants beyond the original POSRPT format. The changes introduce parsing for CSV-formatted messages and improve the handling of tag-based key-value pairs with more flexible parsing logic.
Changes:
- Refactored message parsing to support multiple format variants (POSRPT, POS, OPNORM, INRANG)
- Added support for CSV-formatted position reports
- Extended the decoder to handle additional tags (SPD, UTC, SWN, DWN, etc.) and improved existing tag parsing
- Removed hardcoded preamble matching to enable broader message coverage
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| lib/utils/result_formatter.ts | Added airspeed and mach formatting functions to support new parsed fields |
| lib/plugins/Label_80.ts | Complete refactor of parsing logic to handle multiple message formats with tag-based and CSV parsing methods |
| lib/plugins/Label_80.test.ts | Added test cases for new message variants (POSRPT v2/v3, POS, OPNORM, INRANG) and restructured tests with beforeEach |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@lib/plugins/Label_80.ts`:
- Around line 168-175: The SWND/DWND wind parts are processed per-tag so
wind_speed and wind_direction don't survive until the post-switch check; persist
pending parts on results.raw (e.g., results.raw.pendingWind or similar) when
parsing SWND/DWND (handle both spellings "SWND"/"DWND"), set the pending
windSpeed or windDirection there, and when both values exist build the Wind
object and call ResultFormatter.windData; also construct the waypoint from
results.raw.position (not raw.latitude/raw.longitude) and compute flightLevel
from results.raw.altitude as before, then clear the pending wind state on
results.raw after emitting.
- Around line 184-187: The CSV is passing raw HHMMSS (csvParts[2]) directly to
ResultFormatter.time_of_day, so convertHHMMSSToTod must be used to turn HHMMSS
into TOD seconds first; update the call in Label_80 (where
ResultFormatter.time_of_day is invoked) to pass
convertHHMMSSToTod(parseInt(csvParts[2], 10)) (or
convertHHMMSSToTod(csvParts[2]) if it accepts string), and ensure
convertHHMMSSToTod is imported/available in that module.
🧹 Nitpick comments (1)
lib/plugins/Label_80.test.ts (1)
75-75: Remove debug console.log from tests.🧹 Proposed cleanup
- console.log(JSON.stringify(decodeResult, null, 2));
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@lib/plugins/Label_80.ts`:
- Around line 105-115: In the POS handling branch (case 'POS') guard against a
failed regex match and non-numeric groups before emitting coordinates: check
that posResult and posResult.groups.lat/lng exist and parse to finite numbers
(use Number.isFinite on the parsed values) before computing latitude/longitude
and calling ResultFormatter.position; if the match or numeric conversion fails,
skip emitting the position (or log/return early) to avoid propagating NaN values
from posRegex/posResult/val to ResultFormatter.position.
- Around line 105-115: The 'POS' and 'ETA' switch cases declare consts without
block scope which violates the noSwitchDeclarations rule; wrap the entire case
bodies in braces (e.g., case 'POS': { ... } break; and case 'ETA': { ... }
break;) so posRegex, posResult, lat, lon, position and any ETA-local consts are
block-scoped and avoid TDZ/hoisting issues, preserving the existing logic that
computes position and calls ResultFormatter.position(results, position).
♻️ Duplicate comments (2)
lib/plugins/Label_80.ts (2)
155-162: Wind tags are still discarded.
SWN/DWNare treated as unknown, so wind data is never emitted even when both parts exist.
170-178: CSVtime_of_dayshould use TOD seconds conversion.Other paths normalize via
convertHHMMSSToTod; CSV currently passes raw HHMMSS, causing inconsistent raw output.🛠️ Suggested fix
- ResultFormatter.time_of_day(results, parseInt(csvParts[2], 10)); + ResultFormatter.time_of_day(results, DateTimeUtils.convertHHMMSSToTod(csvParts[2].trim()));
🧹 Nitpick comments (1)
lib/plugins/Label_80.test.ts (1)
52-53: PrefertoBeCloseTofor float coordinates to avoid precision noise.The
35.391999999999996literal is brittle and already flagged by the inline FIXME.♻️ Suggested tweak
- expect(decodeResult.raw.position.latitude).toBe(35.391999999999996); // FIXME?: 35.392 - expect(decodeResult.raw.position.longitude).toBe(-79.372); + expect(decodeResult.raw.position.latitude).toBeCloseTo(35.392, 3); + expect(decodeResult.raw.position.longitude).toBeCloseTo(-79.372, 3);
| case 'POS': | ||
| // don't use decodeStringCoordinates because of different position format | ||
| const posRegex = /^(?<latd>[NS])(?<lat>.+)(?<lngd>[EW])(?<lng>.+)/; | ||
| const posResult = val.match(posRegex); | ||
| const lat = Number(posResult?.groups?.lat) * (posResult?.groups?.latd === 'S' ? -1 : 1); | ||
| const lon = Number(posResult?.groups?.lng) * (posResult?.groups?.lngd === 'W' ? -1 : 1); | ||
| const position = { | ||
| latitude: Number.isInteger(lat) ? lat / 1000 : lat / 100, | ||
| longitude: Number.isInteger(lon) ? lon / 1000 : lon / 100, | ||
| }; | ||
| ResultFormatter.position(results, position); |
There was a problem hiding this comment.
Guard against malformed POS values to avoid NaN coordinates.
If the regex fails, lat/lon become NaN and still get emitted.
🧯 Suggested guard
const posRegex = /^(?<latd>[NS])(?<lat>.+)(?<lngd>[EW])(?<lng>.+)/;
const posResult = val.match(posRegex);
+ if (!posResult?.groups) {
+ ResultFormatter.unknown(results, part, '/');
+ break;
+ }
const lat = Number(posResult?.groups?.lat) * (posResult?.groups?.latd === 'S' ? -1 : 1);🧰 Tools
🪛 Biome (2.1.2)
[error] 107-107: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 108-108: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 109-109: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 110-110: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 111-114: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In `@lib/plugins/Label_80.ts` around lines 105 - 115, In the POS handling branch
(case 'POS') guard against a failed regex match and non-numeric groups before
emitting coordinates: check that posResult and posResult.groups.lat/lng exist
and parse to finite numbers (use Number.isFinite on the parsed values) before
computing latitude/longitude and calling ResultFormatter.position; if the match
or numeric conversion fails, skip emitting the position (or log/return early) to
avoid propagating NaN values from posRegex/posResult/val to
ResultFormatter.position.
🧩 Analysis chain
🏁 Script executed:
git ls-files lib/plugins/Label_80.tsRepository: airframesio/acars-decoder-typescript
Length of output: 103
🏁 Script executed:
cat -n lib/plugins/Label_80.ts | sed -n '100,120p'Repository: airframesio/acars-decoder-typescript
Length of output: 1128
🏁 Script executed:
cat -n lib/plugins/Label_80.ts | sed -n '140,155p'Repository: airframesio/acars-decoder-typescript
Length of output: 763
🏁 Script executed:
cat -n lib/plugins/Label_80.ts | sed -n '117,140p'Repository: airframesio/acars-decoder-typescript
Length of output: 1097
🏁 Script executed:
cat -n lib/plugins/Label_80.ts | grep -A 5 "^[[:space:]]*case '" | head -100Repository: airframesio/acars-decoder-typescript
Length of output: 62
🏁 Script executed:
rg "case '[A-Z]+'" lib/plugins/Label_80.ts -nRepository: airframesio/acars-decoder-typescript
Length of output: 448
🏁 Script executed:
rg "case '[A-Z]+':" lib/plugins/Label_80.ts -A 5 | grep -B 5 "const "Repository: airframesio/acars-decoder-typescript
Length of output: 705
🏁 Script executed:
web_search "Biome noSwitchDeclarations rule"Repository: airframesio/acars-decoder-typescript
Length of output: 130
🌐 Web query:
Biome noSwitchDeclarations rule const declarations case blocks
💡 Result:
Yes — Biome’s lint rule noSwitchDeclarations (same as ESLint’s no-case-declarations) disallows lexical declarations (let, const, function, class) directly inside switch case/default clauses because those bindings are visible across the entire switch but only initialized when their case runs. Fixes: wrap the case body in a block or move the declaration outside the switch. Example:
Invalid:
switch (x) {
case 0:
const v = 1; // error
break;
case 1:
v; // may access uninitialized binding
break;
}
Valid:
switch (x) {
case 0: {
const v = 1;
break;
}
case 1:
// v not visible here
break;
}
Config (biome.json):
{
"linter": { "rules": { "correctness": { "noSwitchDeclarations": "error" } } }
}
Sources: Biome rule docs and ESLint no-case-declarations. [1][2]
Sources
[1] Biome — noSwitchDeclarations rule.
[2] ESLint — no-case-declarations.
🏁 Script executed:
cat -n lib/plugins/Label_80.ts | sed -n '163,180p'Repository: airframesio/acars-decoder-typescript
Length of output: 1122
Wrap case blocks with const declarations in braces to satisfy Biome's noSwitchDeclarations rule.
The 'POS' case (lines 105–116) and 'ETA' case (lines 145–148) declare const variables directly within case blocks without block scope. Biome flags this as an error because declarations are hoisted to the switch scope but only initialized when their case executes, causing temporal dead zone issues. Wrapping each case body in braces fixes this.
🛠️ Suggested fix
case 'POS': {
// don't use decodeStringCoordinates because of different position format
const posRegex = /^(?<latd>[NS])(?<lat>.+)(?<lngd>[EW])(?<lng>.+)/;
const posResult = val.match(posRegex);
const lat = Number(posResult?.groups?.lat) * (posResult?.groups?.latd === 'S' ? -1 : 1);
const lon = Number(posResult?.groups?.lng) * (posResult?.groups?.lngd === 'W' ? -1 : 1);
const position = {
latitude: Number.isInteger(lat) ? lat / 1000 : lat / 100,
longitude: Number.isInteger(lon) ? lon / 1000 : lon / 100,
};
ResultFormatter.position(results, position);
break;
}
...
case 'ETA': {
const hhmm = val.split('.')[0].replace(':', '');
ResultFormatter.eta(results, DateTimeUtils.convertHHMMSSToTod(hhmm));
break;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case 'POS': | |
| // don't use decodeStringCoordinates because of different position format | |
| const posRegex = /^(?<latd>[NS])(?<lat>.+)(?<lngd>[EW])(?<lng>.+)/; | |
| const posResult = val.match(posRegex); | |
| const lat = Number(posResult?.groups?.lat) * (posResult?.groups?.latd === 'S' ? -1 : 1); | |
| const lon = Number(posResult?.groups?.lng) * (posResult?.groups?.lngd === 'W' ? -1 : 1); | |
| const position = { | |
| latitude: Number.isInteger(lat) ? lat / 1000 : lat / 100, | |
| longitude: Number.isInteger(lon) ? lon / 1000 : lon / 100, | |
| }; | |
| ResultFormatter.position(results, position); | |
| case 'POS': { | |
| // don't use decodeStringCoordinates because of different position format | |
| const posRegex = /^(?<latd>[NS])(?<lat>.+)(?<lngd>[EW])(?<lng>.+)/; | |
| const posResult = val.match(posRegex); | |
| const lat = Number(posResult?.groups?.lat) * (posResult?.groups?.latd === 'S' ? -1 : 1); | |
| const lon = Number(posResult?.groups?.lng) * (posResult?.groups?.lngd === 'W' ? -1 : 1); | |
| const position = { | |
| latitude: Number.isInteger(lat) ? lat / 1000 : lat / 100, | |
| longitude: Number.isInteger(lon) ? lon / 1000 : lon / 100, | |
| }; | |
| ResultFormatter.position(results, position); | |
| break; | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 107-107: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 108-108: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 109-109: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 110-110: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 111-114: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In `@lib/plugins/Label_80.ts` around lines 105 - 115, The 'POS' and 'ETA' switch
cases declare consts without block scope which violates the noSwitchDeclarations
rule; wrap the entire case bodies in braces (e.g., case 'POS': { ... } break;
and case 'ETA': { ... } break;) so posRegex, posResult, lat, lon, position and
any ETA-local consts are block-scoped and avoid TDZ/hoisting issues, preserving
the existing logic that computes position and calls
ResultFormatter.position(results, position).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/plugins/Label_80.ts`:
- Around line 170-182: parseCsvFormat currently assumes csvParts[0..8] and
header[0..2] exist which will throw or produce NaN for malformed CSVs; add
explicit bounds checks on csvParts.length and header.length before accessing
indices in parseCsvFormat, e.g. verify header has at least 3 entries before
calling CoordinateUtils.decodeStringCoordinates(header[2]) and ensure csvParts
has the required indices (1,2,4,5,6,7,8) before passing them to ResultFormatter;
when fields are missing, call the appropriate ResultFormatter.unknown/unknownArr
defaults or skip parsing (and avoid calling parseInt/charAt on undefined), and
guard temperature/airspeed/currentFuel parsing with conditional parseInt
fallback to safe values or string defaults. Ensure all referenced symbols
(csvParts, header, ResultFormatter.position, ResultFormatter.temperature,
ResultFormatter.airspeed, ResultFormatter.currentFuel,
CoordinateUtils.decodeStringCoordinates, DateTimeUtils.convertHHMMSSToTod) are
only invoked after the presence checks.
♻️ Duplicate comments (2)
lib/plugins/Label_80.ts (2)
145-148: Wrap ETA case block in braces.Static analysis flags that the
const hhmmdeclaration should be block-scoped.🔧 Proposed fix
- case 'ETA': - const hhmm = val.split('.')[0].replace(':', ''); - ResultFormatter.eta(results, DateTimeUtils.convertHHMMSSToTod(hhmm)); - break; + case 'ETA': { + const hhmm = val.split('.')[0].replace(':', ''); + ResultFormatter.eta(results, DateTimeUtils.convertHHMMSSToTod(hhmm)); + break; + }
105-116: Wrap case block in braces and guard against malformed POS values.Static analysis flags that
constdeclarations in switch cases without braces can be accessed by other cases. Additionally, if the regex fails to match,lat/lonbecomeNaNand are still emitted toResultFormatter.position.🔧 Proposed fix
- case 'POS': - // don't use decodeStringCoordinates because of different position format - const posRegex = /^(?<latd>[NS])(?<lat>.+)(?<lngd>[EW])(?<lng>.+)/; - const posResult = val.match(posRegex); - const lat = Number(posResult?.groups?.lat) * (posResult?.groups?.latd === 'S' ? -1 : 1); - const lon = Number(posResult?.groups?.lng) * (posResult?.groups?.lngd === 'W' ? -1 : 1); - const position = { - latitude: Number.isInteger(lat) ? lat / 1000 : lat / 100, - longitude: Number.isInteger(lon) ? lon / 1000 : lon / 100, - }; - ResultFormatter.position(results, position); - break; + case 'POS': { + // don't use decodeStringCoordinates because of different position format + const posRegex = /^(?<latd>[NS])(?<lat>.+)(?<lngd>[EW])(?<lng>.+)/; + const posResult = val.match(posRegex); + if (!posResult?.groups) { + ResultFormatter.unknown(results, part, '/'); + break; + } + const lat = Number(posResult.groups.lat) * (posResult.groups.latd === 'S' ? -1 : 1); + const lon = Number(posResult.groups.lng) * (posResult.groups.lngd === 'W' ? -1 : 1); + const position = { + latitude: Number.isInteger(lat) ? lat / 1000 : lat / 100, + longitude: Number.isInteger(lon) ? lon / 1000 : lon / 100, + }; + ResultFormatter.position(results, position); + break; + }
🧹 Nitpick comments (7)
lib/plugins/Label_80.test.ts (3)
4-19: Inconsistent indentation throughout the test file.The test structure has inconsistent indentation -
beforeEachand tests inside thedescribeblock should be uniformly indented. Thetest('matches qualifiers'...at line 12 appears to be at the wrong indentation level.🔧 Suggested fix for consistent indentation
describe('Label 80', () => { let plugin : Label_80; - beforeEach(() => { - const decoder = new MessageDecoder(); - plugin = new Label_80(decoder); - }); -test('matches qualifiers', () => { - expect(plugin.decode).toBeDefined(); - expect(plugin.name).toBe('label-80'); - expect(plugin.qualifiers).toBeDefined(); - expect(plugin.qualifiers()).toEqual({ + beforeEach(() => { + const decoder = new MessageDecoder(); + plugin = new Label_80(decoder); + }); + + test('matches qualifiers', () => { + expect(plugin.decode).toBeDefined(); + expect(plugin.name).toBe('label-80'); + expect(plugin.qualifiers).toBeDefined(); + expect(plugin.qualifiers()).toEqual({ labels: ['80'], preambles: [], -}); -}); + }); + });
80-89: Duplicate assertion forfuel_on_board.Line 80 and line 89 both assert the same value for
fuel_on_board. Remove the redundant assertion.🔧 Remove duplicate
expect(decodeResult.raw.altitude).toBe(38007); expect(decodeResult.raw.groundspeed).toBe(334); - expect(decodeResult.raw.fuel_on_board).toBe(105); expect(decodeResult.raw.eta_time).toBe(58920);
63-64: Commented-out wind data assertions indicate incomplete feature.These commented assertions reference
wind_datawhich isn't currently populated by the parser (SWN/DWN tags are marked as unknown). Consider either implementing wind data collection or removing these comments to avoid confusion.Would you like me to help implement wind data collection for the SWN/DWN tags, or open an issue to track this as a follow-up task?
lib/plugins/Label_80.ts (4)
4-4: Unused import:Windis imported but never used.The
Windtype is imported but not referenced anywhere in this file. Remove it or implement wind data collection.🔧 Remove unused import
-import { Wind } from '../types/wind';
29-48: Else block body is not indented.The code inside the
elseblock (lines 30-47) should be indented one level for readability. The closing brace on line 48 is also misaligned.🔧 Fix indentation
} else { - const header = lines[0].trim(); - const headerParts = header.split(','); - for(let i=0; i<headerParts.length-1; i++) { - const moreFields = headerParts[i].split('/'); - for(const more of moreFields) { - this.parseTags(more.trim(), decodeResult); + const header = lines[0].trim(); + const headerParts = header.split(','); + for(let i=0; i<headerParts.length-1; i++) { + const moreFields = headerParts[i].split('/'); + for(const more of moreFields) { + this.parseTags(more.trim(), decodeResult); + } } - } - this.parseHeader(headerParts[headerParts.length-1].trim(), decodeResult); + this.parseHeader(headerParts[headerParts.length-1].trim(), decodeResult); - for(let i=1; i<lines.length; i++) { - const line = lines[i]; - const parts = line.split('/'); - for (const part of parts) { - this.parseTags(part.trim(), decodeResult); + for(let i=1; i<lines.length; i++) { + const line = lines[i]; + const parts = line.split('/'); + for (const part of parts) { + this.parseTags(part.trim(), decodeResult); + } } } - }
58-58: UseDecodeResulttype instead ofany.For type safety and consistency with
parseTags, useDecodeResultinstead ofany.🔧 Fix type annotation
- private parseHeader(header: string, results: any) { + private parseHeader(header: string, results: DecodeResult) {
173-181: Inconsistent spacing in function calls.Minor formatting inconsistencies: missing spaces after commas on lines 173-174, and extra spaces inside parentheses on line 181.
🔧 Fix spacing
- ResultFormatter.unknown(results,header[0], ' '); - ResultFormatter.unknown(results,header[1], ' '); + ResultFormatter.unknown(results, header[0], ' '); + ResultFormatter.unknown(results, header[1], ' '); ResultFormatter.position(results, CoordinateUtils.decodeStringCoordinates(header[2])); ResultFormatter.unknown(results, csvParts[1]); ResultFormatter.time_of_day(results, DateTimeUtils.convertHHMMSSToTod(csvParts[2])); ResultFormatter.unknownArr(results, csvParts.slice(4,6), ','); ResultFormatter.temperature(results, ((csvParts[6].charAt(0) === 'M' ? -1 : 1) * parseInt(csvParts[6].slice(1), 10)).toString()); ResultFormatter.airspeed(results, parseInt(csvParts[7], 10)); - ResultFormatter.currentFuel( results, parseInt( csvParts[8], 10)); + ResultFormatter.currentFuel(results, parseInt(csvParts[8], 10));
| private parseCsvFormat(text: string, results: DecodeResult) { | ||
| const csvParts = text.split(','); | ||
| const header = csvParts[0].trim().split(/\s+/); | ||
| ResultFormatter.unknown(results,header[0], ' '); | ||
| ResultFormatter.unknown(results,header[1], ' '); | ||
| ResultFormatter.position(results, CoordinateUtils.decodeStringCoordinates(header[2])); | ||
| ResultFormatter.unknown(results, csvParts[1]); | ||
| ResultFormatter.time_of_day(results, DateTimeUtils.convertHHMMSSToTod(csvParts[2])); | ||
| ResultFormatter.unknownArr(results, csvParts.slice(4,6), ','); | ||
| ResultFormatter.temperature(results, ((csvParts[6].charAt(0) === 'M' ? -1 : 1) * parseInt(csvParts[6].slice(1), 10)).toString()); | ||
| ResultFormatter.airspeed(results, parseInt(csvParts[7], 10)); | ||
| ResultFormatter.currentFuel( results, parseInt( csvParts[8], 10)); | ||
| } |
There was a problem hiding this comment.
Missing bounds checking in CSV parsing.
The method accesses csvParts[0] through csvParts[8] and header[0] through header[2] without verifying these indices exist. Malformed input with fewer fields will cause undefined access, leading to NaN values or runtime errors.
🔧 Add validation
private parseCsvFormat(text: string, results: DecodeResult) {
const csvParts = text.split(',');
+ if (csvParts.length < 9) {
+ ResultFormatter.unknown(results, text, ',');
+ return;
+ }
const header = csvParts[0].trim().split(/\s+/);
+ if (header.length < 3) {
+ ResultFormatter.unknown(results, text, ',');
+ return;
+ }
ResultFormatter.unknown(results,header[0], ' ');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private parseCsvFormat(text: string, results: DecodeResult) { | |
| const csvParts = text.split(','); | |
| const header = csvParts[0].trim().split(/\s+/); | |
| ResultFormatter.unknown(results,header[0], ' '); | |
| ResultFormatter.unknown(results,header[1], ' '); | |
| ResultFormatter.position(results, CoordinateUtils.decodeStringCoordinates(header[2])); | |
| ResultFormatter.unknown(results, csvParts[1]); | |
| ResultFormatter.time_of_day(results, DateTimeUtils.convertHHMMSSToTod(csvParts[2])); | |
| ResultFormatter.unknownArr(results, csvParts.slice(4,6), ','); | |
| ResultFormatter.temperature(results, ((csvParts[6].charAt(0) === 'M' ? -1 : 1) * parseInt(csvParts[6].slice(1), 10)).toString()); | |
| ResultFormatter.airspeed(results, parseInt(csvParts[7], 10)); | |
| ResultFormatter.currentFuel( results, parseInt( csvParts[8], 10)); | |
| } | |
| private parseCsvFormat(text: string, results: DecodeResult) { | |
| const csvParts = text.split(','); | |
| if (csvParts.length < 9) { | |
| ResultFormatter.unknown(results, text, ','); | |
| return; | |
| } | |
| const header = csvParts[0].trim().split(/\s+/); | |
| if (header.length < 3) { | |
| ResultFormatter.unknown(results, text, ','); | |
| return; | |
| } | |
| ResultFormatter.unknown(results,header[0], ' '); | |
| ResultFormatter.unknown(results,header[1], ' '); | |
| ResultFormatter.position(results, CoordinateUtils.decodeStringCoordinates(header[2])); | |
| ResultFormatter.unknown(results, csvParts[1]); | |
| ResultFormatter.time_of_day(results, DateTimeUtils.convertHHMMSSToTod(csvParts[2])); | |
| ResultFormatter.unknownArr(results, csvParts.slice(4,6), ','); | |
| ResultFormatter.temperature(results, ((csvParts[6].charAt(0) === 'M' ? -1 : 1) * parseInt(csvParts[6].slice(1), 10)).toString()); | |
| ResultFormatter.airspeed(results, parseInt(csvParts[7], 10)); | |
| ResultFormatter.currentFuel( results, parseInt( csvParts[8], 10)); | |
| } |
🤖 Prompt for AI Agents
In `@lib/plugins/Label_80.ts` around lines 170 - 182, parseCsvFormat currently
assumes csvParts[0..8] and header[0..2] exist which will throw or produce NaN
for malformed CSVs; add explicit bounds checks on csvParts.length and
header.length before accessing indices in parseCsvFormat, e.g. verify header has
at least 3 entries before calling
CoordinateUtils.decodeStringCoordinates(header[2]) and ensure csvParts has the
required indices (1,2,4,5,6,7,8) before passing them to ResultFormatter; when
fields are missing, call the appropriate ResultFormatter.unknown/unknownArr
defaults or skip parsing (and avoid calling parseInt/charAt on undefined), and
guard temperature/airspeed/currentFuel parsing with conditional parseInt
fallback to safe values or string defaults. Ensure all referenced symbols
(csvParts, header, ResultFormatter.position, ResultFormatter.temperature,
ResultFormatter.airspeed, ResultFormatter.currentFuel,
CoordinateUtils.decodeStringCoordinates, DateTimeUtils.convertHHMMSSToTod) are
only invoked after the presence checks.
1 participant
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.