chore(preprod): Include idiom and colorspace info in asssets.json#2860
chore(preprod): Include idiom and colorspace info in asssets.json#2860NicoHinderling merged 1 commit intomasterfrom
Conversation
| let idiomValue = key.getUInt(forKey: "themeIdiom") | ||
| let colorSpaceID = rendition.getUInt(forKey: "colorSpaceID") |
There was a problem hiding this comment.
Bug: The getUInt extension uses perform(Selector(...)) without exception handling, which can cause a crash if the selector does not exist on the target object.
(Severity: Critical 0.85 | Confidence: 0.95)
🔍 Detailed Analysis
The getUInt(forKey:) extension method is called on lines 167 and 168 to retrieve themeIdiom and colorSpaceID. This method uses perform(Selector(key)) without a try-catch block. If the key or rendition objects, which are from Apple's private frameworks, do not implement methods corresponding to these selectors, the application will crash with an "Unrecognized Selector" exception. This can happen with different asset types or macOS versions. The codebase already contains a safeValueForKey function that handles this exact scenario, but it is not used for these new calls.
💡 Suggested Fix
Modify the getUInt extension to handle cases where the selector does not exist. Either wrap the perform(Selector(key)) call in a try-catch block or check responds(to:) before making the call to prevent a crash.
🤖 Prompt for AI Agent
Fix this bug. In
apple-catalog-parsing/native/swift/AssetCatalogParser/Sources/AssetCatalogParser/AssetCatalogReader.swift
at lines 167-168: The `getUInt(forKey:)` extension method is called on lines 167 and 168
to retrieve `themeIdiom` and `colorSpaceID`. This method uses `perform(Selector(key))`
without a `try-catch` block. If the `key` or `rendition` objects, which are from Apple's
private frameworks, do not implement methods corresponding to these selectors, the
application will crash with an "Unrecognized Selector" exception. This can happen with
different asset types or macOS versions. The codebase already contains a
`safeValueForKey` function that handles this exact scenario, but it is not used for
these new calls.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
The crash risk would only be if the selector itself didn't exist, but these are standard CoreUI selectors that do exist on these objects
So we can show users why they have duplicate images despite not having intended to (and nudge them to consider app thinning on their side)