-
Notifications
You must be signed in to change notification settings - Fork 1
[SPLIT] parseCollectionResponse casts raw JSON to T[] without element validation → #154 + #155 #141
Description
⚠️ This issue has been split into two parts for safe execution
Per the Phase 7 Scope & Split Assessment, this issue has ~45 call sites across 7 test files that each require semantic judgment (choosing the correct parser callback per resource type). Doing the implementation + all test updates in one pass exceeds safe single-pass capacity.
Split Issues
| Part | Issue | Scope |
|---|---|---|
| Part 1 | #154 | Implement parseItem callback in parseCollectionResponse + unit tests (response.ts, response.spec.ts) |
| Part 2 | #155 | Update all integration test call sites (discovery.spec.ts, command.spec.ts, navigation.spec.ts, observation.spec.ts, pipeline.spec.ts) |
Execution Order
- #141 Part 1: Implement
parseItemcallback inparseCollectionResponse+ unit tests #154 (Part 1) — implement the core change - #141 Part 2: Update integration test call sites for
parseCollectionResponseparseItemcallback #155 (Part 2) — fix the integration test call sites
This parent issue should be closed when both #154 and #155 are resolved.
Original issue body (preserved for reference)
Finding
parseCollectionResponse casts the raw features or items array directly to T[] without validating any element. The function promises CollectionResponse<T> but delivers CollectionResponse<unknown> masked as typed. Every downstream caller receives data it believes is typed but which may contain null, numbers, or malformed objects.
Review Source: Senior developer code review of clean-pr — docs/code-review/003-pending-p1-unchecked-generic-cast-response.md
Severity: P1-Critical
Category: Type Safety
Ownership: Ours
Problem Statement
parseCollectionResponse is the single boundary where untyped server JSON enters the CSAPI typed system. If this boundary is dishonest, all of CSAPI's type safety downstream is illusory. Array.isArray confirms an array exists but says nothing about element types. The as T[] cast tells TypeScript the elements are T without any runtime verification.
Affected code:
// src/ogc-api/csapi/formats/response.ts, lines 98–103
let items: T[];
if (Array.isArray(obj.features)) {
items = obj.features as T[]; // ← no element validation
} else if (Array.isArray(obj.items)) {
items = obj.items as T[]; // ← same
}Scenario:
// Server returns { "items": [null, 42, { "no-id": true }] }
const result = parseCollectionResponse<Datastream>(body);
result.items[0].id // TypeError: Cannot read properties of null
result.items[1].id // TypeError: undefined is not a property of number
// TypeScript shows no warning — it believes items are all DatastreamImpact: All Part 2 parsers and integration tests that call parseCollectionResponse receive unvalidated items. If a server returns malformed elements, runtime TypeErrors occur at field access with no TS warning at the call site. Currently ~45 call sites across 7 test files (response.spec.ts, discovery.spec.ts, command.spec.ts, navigation.spec.ts, observation.spec.ts, pipeline.spec.ts, index.spec.ts). There are no production (non-test) call sites — the function is exported via formats/index.ts for consumer use but is not called internally by other production code.