Skip to content

Bug: parseIcaoFpl silently swaps departure/destination ICAO into the time field when the field has no trailing time #455

Description

@kevinelliott

Summary

parseIcaoFpl extracts the ICAO + time portions of the departure and destination sections with a fixed-offset slice (field.length - 4). The function never validates that the field is long enough to contain both pieces. When a field omits the trailing time (e.g., a malformed FPL with just KSFO and no departure time, or just EGLL with no EET), JavaScript's substring clamps negative arguments to 0, so the ICAO is silently moved into the departureTime / eet slot and the ICAO field becomes empty — instead of the function returning null to signal "not a well-formed FPL".

Affected code

Same shape, two places: lib/utils/icao_fpl_utils.ts:113-117 (departure) and lib/utils/icao_fpl_utils.ts:136-142 (destination).

// Section 5: departure aerodrome + time (e.g., "VESPA2354" or "KSFO0100")
const deptField = parts[4].trim();
// Time is always last 4 digits
const departureTime = deptField.substring(deptField.length - 4);
const departure     = deptField.substring(0, deptField.length - 4);

// ...

// Section 7: destination + EET + alternates (e.g., "KLAX0117 KSFO")
const destField  = parts[6].trim();
const destParts  = destField.split(/\s+/);
const destTime   = destParts[0];
// Destination ICAO is first 4 chars, EET is last 4 digits
const destination = destTime.substring(0, destTime.length - 4);
const eet         = destTime.substring(destTime.length - 4);

Reproduction

// Hand-rolled FPL where the destination section is just an ICAO, no EET.
parseIcaoFpl('(FPL-AAL123-IS-B77W/H-SDE3FGHIJ1J2J3J4J5M1P2RWXYZ/LB1D1-KSFO0100-N0482F350 DCT ENI-KLAX-PBN/A1B1)');
// returns:
//   destination: ''
//   eet:         'KLAX'
//
// And if the departure field is also short (e.g. just "KSFO"):
//   departure:     ''
//   departureTime: 'KSFO'

In both cases the function happily returns an IcaoFlightPlan object with the airport ICAO stored as a time, which will silently corrupt anything that consumes result.destination / result.departure (route display, downstream airport lookups, etc.).

Suggested fix

Guard the field length before the split, and treat anything shorter than the minimum ICAO+time width as a malformed FPL by returning null (consistent with the other validation early-returns in the same function):

// Section 5
const deptField = parts[4].trim();
if (deptField.length < 8 || !/\d{4}$/.test(deptField)) {
  return null;
}
const departureTime = deptField.slice(-4);
const departure     = deptField.slice(0, -4);

// Section 7
const destTime = destParts[0];
if (destTime.length < 8 || !/\d{4}$/.test(destTime)) {
  return null;
}
const destination = destTime.slice(0, -4);
const eet         = destTime.slice(-4);

Trailing-\d{4} check also defends against KLAXabcd (4 ICAO + 4 non-digits) — the existing code would accept that and store "abcd" as EET.

Test coverage

The existing tests in icao_fpl_utils.test.ts only cover well-formed inputs (KSFO0100, KLAX0117). Adding a couple of malformed-input cases that assert parseIcaoFpl(...) returns null (or your preferred sentinel) would catch this regressing again.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions