Skip to content

feat: Added AvatarType to Bitmap logic in core.coil#742

Closed
amariaux wants to merge 1 commit intomainfrom
avatarType-to-bitmap
Closed

feat: Added AvatarType to Bitmap logic in core.coil#742
amariaux wants to merge 1 commit intomainfrom
avatarType-to-bitmap

Conversation

@amariaux
Copy link
Copy Markdown

@amariaux amariaux requested a review from benjaminVadon April 13, 2026 14:25
amariaux pushed a commit to Infomaniak/android-kMail that referenced this pull request Apr 13, 2026
@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 Security concerns

Sensitive information exposure:
The error logging in loadAsBitmap includes the full URL ($url) in the Sentry log message. If avatar URLs contain authentication tokens, API keys, or other sensitive query parameters in the URL, these secrets will be transmitted to and stored in Sentry. This violates security best practices for handling URLs that may contain credentials or signed access tokens.

⚡ Recommended focus areas for review

Inconsistent Bitmap Sizes

The loadAsBitmap function uses a default size of 256, but the fallback initials generation in AvatarType.WithInitials.Url and AvatarType.WithInitials.Initials uses the default size of 64 from generateInitialsAvatarDrawable. This results in inconsistent bitmap dimensions (256x256 for successful network loads vs 64x64 for initials fallback), potentially causing UI scaling issues or blurry avatars.

suspend fun AvatarType.toBitmap( appContext : Context): Bitmap? = when (this) {
    is AvatarType.WithInitials.Url -> {
        loadAsBitmap(url = this.url, appContext = appContext) ?: appContext.generateInitialsAvatarDrawable(
            initials = this.initials,
            background = getBackgroundColorGradientDrawable(this.colors.containerColor.toArgb()),
            initialsColor = this.colors.contentColor.toArgb(),
        ).toBitmap()
    }
    is AvatarType.WithInitials.Initials -> {
        appContext.generateInitialsAvatarDrawable(
            initials = this.initials,
            background = getBackgroundColorGradientDrawable(this.colors.containerColor.toArgb()),
            initialsColor = this.colors.contentColor.toArgb(),
        ).toBitmap()
    }
    is AvatarType.DrawableResource -> {
        ContextCompat.getDrawable(appContext, this.resource)?.toBitmap()
    }
    else -> null
}
Potential Information Disclosure

The loadAsBitmap function logs the full URL to Sentry on error ("Failed to load SVG avatar: $url"). If avatar URLs contain sensitive query parameters (e.g., authentication tokens, signed URLs with secrets), these will be exposed in Sentry logs. Consider logging only the domain or a sanitized version of the URL.

SentryLog.e(TAG, "Failed to load SVG avatar: $url", result.throwable)

@github-actions
Copy link
Copy Markdown

Failed to generate code suggestions for PR

@amariaux amariaux force-pushed the avatarType-to-bitmap branch from 87985f2 to 8dc403d Compare April 13, 2026 14:45
@sonarqubecloud
Copy link
Copy Markdown

@github-actions
Copy link
Copy Markdown

Failed to generate code suggestions for PR

@amariaux amariaux closed this Apr 15, 2026
@amariaux amariaux deleted the avatarType-to-bitmap branch April 15, 2026 08:13
aymericmariaux added a commit to Infomaniak/android-kMail that referenced this pull request Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants