Skip to content

fix: add logical to handle local image asset #192

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
uc-brunosouza wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: add logical to handle local image asset #192

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 31 additions & 6 deletions ios/Extensions/UIImage+UsercentricsLogoDict.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,38 @@ import UIKit

extension UIImage {
convenience init?(from dictionary: NSDictionary?) {
guard
let dictionary = dictionary,
let logoName = dictionary["logoName"] as? String
else {
return nil
guard let dictionary = dictionary else { return nil }

if let logoPath = dictionary["logoPath"] as? String, !logoPath.isEmpty,
let image = Self.image(fromLogoPath: logoPath),
let cgImage = image.cgImage {
self.init(cgImage: cgImage, scale: image.scale, orientation: image.imageOrientation)
return
}

if let logoName = dictionary["logoName"] as? String, !logoName.isEmpty {
self.init(named: logoName)
return
}

self.init(named: logoName)
return nil
}

private static func image(fromLogoPath logoPath: String) -> UIImage? {
if logoPath.hasPrefix("file://") {
let path = logoPath.replacingOccurrences(of: "file://", with: "")
if let image = UIImage(contentsOfFile: path) { return image }
Comment on lines +24 to +26
Copy link

@codeant-ai codeant-ai bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Stripping file:// via string replacement can produce invalid filesystem paths for percent-encoded file URLs (for example spaces encoded as %20), causing local images to fail to load. Parse the value as a URL and use its .path to get a valid decoded file path. [logic error]

Severity Level: Major ⚠️ 
- ⚠️ Local `file://` logos may not render reliably.
- ⚠️ Banner branding can disappear for encoded file paths.
Suggested change
if logoPath.hasPrefix("file://") {
let path = logoPath.replacingOccurrences(of: "file://", with: "")
if let image = UIImage(contentsOfFile: path) { return image }
if logoPath.hasPrefix("file://"), let fileURL = URL(string: logoPath) {
if let image = UIImage(contentsOfFile: fileURL.path) { return image }
Steps of Reproduction ✅ 
1. Build banner settings with a logo dictionary path that starts with `file://` (logo
parsing entrypoint: `ios/Extensions/BannerSettings+Dict.swift:36`, `123-124`).

2. Pass settings through normal banner APIs (`src/Usercentrics.tsx:38-45` ->
`ios/RNUsercentricsModule.swift:82`/`97` -> `BannerSettings(from:)`).

3. When `logoPath` contains URL-escaped characters (e.g. `%20`), current code strips
prefix by string replacement at `ios/Extensions/UIImage+UsercentricsLogoDict.swift:24-26`
and feeds encoded path to `UIImage(contentsOfFile:)`.

4. Image load fails for that path format; using `URL(string:).path` decodes correctly and
restores local-file loading.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** ios/Extensions/UIImage+UsercentricsLogoDict.swift
**Line:** 24:26
**Comment:**
	*Logic Error: Stripping `file://` via string replacement can produce invalid filesystem paths for percent-encoded file URLs (for example spaces encoded as `%20`), causing local images to fail to load. Parse the value as a URL and use its `.path` to get a valid decoded file path.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

}
else if logoPath.hasPrefix("/") {
if let image = UIImage(contentsOfFile: logoPath) { return image }
}
else if logoPath.hasPrefix("http://") || logoPath.hasPrefix("https://"),
Copy link

@codeant-ai codeant-ai bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The HTTP/HTTPS branch performs a synchronous network read with Data(contentsOf:), and this initializer is invoked from the main queue during banner setup. That can block the UI thread and freeze banner presentation. Avoid synchronous network loading in this path (or at least skip it on the main thread) and only use already-local image sources here. [possible bug]

Severity Level: Major ⚠️ 
- ❌ Banner presentation can freeze during remote logo fetch.
- ⚠️ `showFirstLayer`/`showSecondLayer` responsiveness degrades on slow networks.
Suggested change
else if logoPath.hasPrefix("http://") || logoPath.hasPrefix("https://"),
else if (logoPath.hasPrefix("http://") || logoPath.hasPrefix("https://")),
!Thread.isMainThread,
Steps of Reproduction ✅ 
1. Trigger banner display via public API `Usercentrics.showFirstLayer(options)` in
`src/Usercentrics.tsx:38-40`.

2. Native bridge executes `RNUsercentricsModule.showFirstLayer` on `DispatchQueue.main`
(`ios/RNUsercentricsModule.swift:23`, `73-85`), then builds `BannerSettings(from:)`.

3. Settings parsing calls `UIImage(from:)` through `GeneralStyleSettings`
(`ios/Extensions/BannerSettings+Dict.swift:28-37`), then `image(fromLogoPath:)`
(`ios/Extensions/UIImage+UsercentricsLogoDict.swift:23-35`).

4. Provide `logo.logoPath` as `https://...` (same serialized dictionary flow used in tests
at `sample/ios/sampleTests/RNUsercentricsModuleTests.swift:485`); `Data(contentsOf:)` runs
synchronously on main thread and can stall banner rendering/UI.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** ios/Extensions/UIImage+UsercentricsLogoDict.swift
**Line:** 31:31
**Comment:**
	*Possible Bug: The HTTP/HTTPS branch performs a synchronous network read with `Data(contentsOf:)`, and this initializer is invoked from the main queue during banner setup. That can block the UI thread and freeze banner presentation. Avoid synchronous network loading in this path (or at least skip it on the main thread) and only use already-local image sources here.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

let url = URL(string: logoPath),
let data = try? Data(contentsOf: url),
let image = UIImage(data: data) {
return image
Comment on lines +31 to +35
Copy link

@qodo-code-review qodo-code-review bot Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Main-thread image download 🐞 Bug ➹ Performance

UIImage.image(fromLogoPath:) performs a synchronous Data(contentsOf:) fetch for http(s) paths,
and this logo parsing runs on DispatchQueue.main during showFirstLayer/showSecondLayer. A remote
logoPath will block the UI thread while downloading, potentially freezing the app and triggering
watchdog termination.
Agent Prompt 
## Issue description
`UIImage.image(fromLogoPath:)` uses `Data(contentsOf:)` for `http(s)` logo paths. Because `BannerSettings(from:)` is built on `DispatchQueue.main` in `RNUsercentricsModule.showFirstLayer/showSecondLayer`, a remote `logoPath` will synchronously block the main thread.

## Issue Context
- `RNUsercentricsModule.queue` is set to `DispatchQueue.main`, and `BannerSettings(from: dict)` is created inside `queue.async`.
- `BannerSettings` parsing calls `UIImage(from:)` for the logo.
- `UIImage(from:)` calls `image(fromLogoPath:)`, which includes an `http(s)` branch using synchronous `Data(contentsOf:)`.

## Fix Focus Areas
- ios/Extensions/UIImage+UsercentricsLogoDict.swift[31-36]
- ios/RNUsercentricsModule.swift[22-85]
- ios/Extensions/BannerSettings+Dict.swift[27-48]

## Suggested fix approach
- Do not perform any network I/O inside `UIImage` initializers.
- Either:
  1) Remove the `http(s)` branch entirely (treat remote URLs as unsupported for `logoPath`), and rely on a separate async loader elsewhere, or
  2) Redesign the logo-loading flow to be asynchronous (e.g., pass `URL`/string through settings, fetch via `URLSession` off-main-thread, then update the banner UI when the image arrives).
- If you must keep remote support, ensure the fetch happens off the main thread and has timeouts/error handling, and avoid blocking the caller.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}
Comment on lines +31 to +36
Copy link

@pantoaibot pantoaibot bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CRITICAL_BUG] The implementation performs a synchronous network fetch using Data(contentsOf:) (lines 31-36). This will block the calling thread (very likely the main thread) and can freeze the UI or cause the app to be terminated for unresponsiveness. Move remote image loading off the calling thread and make it asynchronous (URLSession dataTask or use an image-loading/caching library). Also avoid initiating network I/O from a synchronous convenience initializer — consider providing an asynchronous factory/completion-based API (e.g. loadImage(from:completion:)) or return a placeholder and fetch the remote image asynchronously.

private static func image(fromLogoPath logoPath: String) -> UIImage? {
    if logoPath.hasPrefix("file://") {
        let path = logoPath.replacingOccurrences(of: "file://", with: "")
        if let image = UIImage(contentsOfFile: path) { return image }
    } else if logoPath.hasPrefix("/") {
        if let image = UIImage(contentsOfFile: logoPath) { return image }
    } else if logoPath.hasPrefix("http://") || logoPath.hasPrefix("https://"),
              let url = URL(string: logoPath) {
        // Avoid blocking the caller here; return a placeholder instead and
        // let the caller load the remote image asynchronously.
        return UIImage(named: logoPath)
    }

    if let image = UIImage(named: logoPath) { return image }
    return nil
}

// Example async loader (outside the UIImage extension):
func loadLogo(from logoPath: String, completion: @escaping (UIImage?) -> Void) {
    if logoPath.hasPrefix("http://") || logoPath.hasPrefix("https://"),
       let url = URL(string: logoPath) {
        URLSession.shared.dataTask(with: url) { data, _, _ in
            let image = data.flatMap { UIImage(data: $0) }
            DispatchQueue.main.async {
                completion(image)
            }
        }.resume()
    } else {
        completion(UIImage.image(fromLogoPath: logoPath))
    }
}

Comment on lines +31 to +36
Copy link

@coderabbitai coderabbitai bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Synchronous network request will block the calling thread.

Data(contentsOf: url) performs a blocking network call. If the initializer is invoked on the main thread (typical for UI setup code), this will freeze the UI and could trigger watchdog termination on slow or unreachable networks.

Consider one of the following approaches:

  1. Load remote images asynchronously and update the UI via a completion handler or reactive binding.
  2. If synchronous loading is required, document that this must not be called from the main thread and add an assertion during development.
  3. Add a timeout to prevent indefinite hangs.
🛡️ Minimal defensive fix: assert off main thread + timeout 
         else if logoPath.hasPrefix("http://") || logoPath.hasPrefix("https://"),
-                let url = URL(string: logoPath),
-                let data = try? Data(contentsOf: url),
-                let image = UIImage(data: data) {
+                let url = URL(string: logoPath) {
+            `#if` DEBUG
+            assert(!Thread.isMainThread, "Loading remote image synchronously on main thread is not allowed")
+            `#endif`
+            var request = URLRequest(url: url, cachePolicy: .returnCacheDataElseLoad, timeoutInterval: 10)
+            if let data = try? Data(contentsOf: url, options: .uncached) {
+                if let image = UIImage(data: data) { return image }
+            }
             return image
         }

Note: A fully async solution is preferred but may require API changes to the initializer.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@ios/Extensions/UIImage`+UsercentricsLogoDict.swift around lines 31 - 36, The
code currently blocks using Data(contentsOf: url) when logoPath is an http(s)
URL; replace this synchronous network call in the UIImage+UsercentricsLogoDict
extension: either (preferred) change the API to load remote images
asynchronously (add a helper like loadRemoteImageAsync(url:completion:) that
uses URLSession.shared.dataTask(with: URLRequest(url: url, timeoutInterval: X))
and invokes the completion on the main thread to set the image, or (minimal
defensive) assert(!Thread.isMainThread) in the http(s) branch and perform a
synchronous fetch with URLSession and a semaphore/timeout to avoid indefinite
hangs; update the branch that checks logoPath.hasPrefix("http://") ||
logoPath.hasPrefix("https://") to use the chosen approach and remove
Data(contentsOf: url).

if let image = UIImage(named: logoPath) { return image }
return nil
}
}
Loading