refactor: move attribute projection parsing to Swift - #927
nickolas-dimitrakas merged 1 commit into
Conversation
🐦 Swift Migration ProgressProduction implementation code at
Objective-C retained by design: Core SDK 5,802 · SDK kit infrastructure 2,041 · Standalone kits 0. This PR's code movement
How this is measured
Generated with |
📦 SDK Size Impact ReportMeasures how much the SDK adds to an app's size (with-SDK minus without-SDK).
➡️ SDK size impact change is minimal. Raw measurementsTarget branch (workstation/swift-migration): {"baseline_app_size_kb":84,"baseline_executable_size_bytes":75464,"with_sdk_app_size_kb":2600,"with_sdk_executable_size_bytes":76312,"sdk_impact_kb":2516,"sdk_executable_impact_bytes":848,"xcframework_size_kb":6968}This PR: {"baseline_app_size_kb":84,"baseline_executable_size_bytes":75464,"with_sdk_app_size_kb":2616,"with_sdk_executable_size_bytes":76312,"sdk_impact_kb":2532,"sdk_executable_impact_bytes":848,"xcframework_size_kb":6984} |
95134f2 to
ad8a9e1
Compare
PR SummaryMedium Risk Overview Fixes crashes on malformed remote config: null or non-dictionary
Reviewed by Cursor Bugbot for commit 013280f. Bugbot is set up for automated code reviews on this repo. Configure here. |
MPAttributeProjection was the one file the kit projection series never opened — no Swift import, unchanged since 2018 — and it still walked configuration["action"]["attribute_maps"][index] itself, duplicating navigation MPProjectionFieldParser already does with type guards. data_type and is_required now come from +attributeProjectionFieldsFromConfiguration:attributeIndex:, which reuses the same private navigation as attributeFieldsFromAction:attributeIndex: so the two readers cannot drift, and goes through MPJSONCoercion so a string-encoded value behaves as -integerValue and -boolValue did. -setDataType: keeps its range clamp and stays the single source of it; Swift returns the raw value. Fixes a crash reachable from remote configuration: attribute_maps: [null] crashed with -[NSNull objectForKeyedSubscript:], because the element was subscripted with no guard on the container, the index, or the element type. Unreadable entries now fall back to the same defaults the absent-key path always used. Also bounds-guards configureProjections:, where message_type is unvalidated remote configuration used as an array index. Note that -setObject:atIndexedSubscript: allows index == count and appends, so MPMessageTypeMedia (20) has always been registered into a 21st slot even though +[MPEnum messageTypeSize] is 20 — that case is deliberately preserved, and a test pins it so the guard cannot regress Media projections. Only a larger value, or a negative one that wrapped, raises NSRangeException; those are now skipped and logged. The messageTypeSize/MPMessageTypeMedia off-by-one itself is left alone: making Media an explicit slot is a behaviour change that needs product input. Not done: the dead-looking early return in -setFilters:. `filters` is a readwrite property, so that short-circuit is reachable by any holder of the header, not dead code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ad8a9e1 to
013280f
Compare
7982b6e
into
workstation/swift-migration
Background
MPAttributeProjectionwas the last unmigrated file in the projection/filter series, duplicating navigation logicMPProjectionFieldParseralready implements.What Has Changed
attributeProjectionFields(from:attributeIndex:)toMPProjectionFieldParser.swift, reusing the existing private navigation so the two readers can't drift; coerces string-encodeddata_type/is_requiredviaMPJSONCoercion.attribute_mapshit-[NSNull objectForKeyedSubscript:]; unreadable entries now fall back to the existing absent-key defaults.configureProjections:for the unvalidated remotemessage_typeindex — preserves the existing (working) off-by-one that letsMediaprojections register, pinned by a new test.-setDataType:'s range clamp stays in ObjC (asserted by an existing test).Validation
abi-guardPASSED (no public ABI diff)trunk checkcleanChecklist