Return data uri image as icon support #3104
Conversation
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughThe changes involve modifications to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
Flow.Launcher.Infrastructure/Image/ImageLoader.cs (2)
Line range hint
142-147: Enhance data URI handling with validation and size limits.The current implementation needs additional safeguards:
- Validate data URI format
- Add size limits to prevent memory issues
- Consider caching data URI images
Apply these improvements:
-if (path.StartsWith("data:image", StringComparison.OrdinalIgnoreCase)) +if (path.StartsWith("data:image/", StringComparison.OrdinalIgnoreCase)) { + const int MaxDataUriLength = 5 * 1024 * 1024; // 5MB limit + if (path.Length > MaxDataUriLength) + throw new ArgumentException($"Data URI exceeds maximum size of {MaxDataUriLength} bytes"); + + // Validate data URI format + var parts = path.Split(','); + if (parts.Length != 2 || !parts[0].EndsWith(";base64", StringComparison.OrdinalIgnoreCase)) + throw new ArgumentException("Invalid data URI format"); + + try + { + Convert.FromBase64String(parts[1]); + } + catch (FormatException) + { + throw new ArgumentException("Invalid base64 data in URI"); + } + var imageSource = new BitmapImage(new Uri(path)); + if (!loadFullImage) + { + imageSource.DecodePixelHeight = SmallIconSize; + imageSource.DecodePixelWidth = SmallIconSize; + } imageSource.Freeze(); + ImageCache[path, loadFullImage] = imageSource; return new ImageResult(imageSource, ImageType.Data); }
Line range hint
148-163: Well-implemented retry mechanism for thumbnail loading.The retry logic with proper exception logging improves reliability. Consider adding a brief delay between retries for transient issues.
Consider adding a delay between retries:
catch (System.Exception e) { try { + await Task.Delay(100); // Add brief delay before retry imageResult = GetThumbnailResult(ref path, loadFullImage); } catch (System.Exception e2) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
Flow.Launcher.Infrastructure/Image/ImageLoader.cs(1 hunks)Flow.Launcher.Plugin/Result.cs(1 hunks)
🔇 Additional comments (1)
Flow.Launcher.Infrastructure/Image/ImageLoader.cs (1)
Line range hint 142-147: Verify data URI usage across the codebase.
Let's ensure there are no existing data URI implementations that might conflict with these changes.
✅ Verification successful
Based on the results, I can now generate the final response:
Data URI implementation is properly isolated and consistent
The search results show that:
- Data URI usage is properly contained within the image loading infrastructure
- The new implementation aligns with existing
BitmapImageusage patterns across the codebase - No conflicting implementations or memory-related TODOs were found for data URI handling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for existing data URI usage
echo "Searching for data URI usage in code..."
rg -l "data:image"
# Search for potential image loading implementations
echo "Searching for image loading implementations..."
ast-grep --pattern 'new BitmapImage($$$)'
# Search for potential memory/performance related issues
echo "Searching for TODO/FIXME comments related to images..."
rg -i "todo|fixme|hack" -A 3 -g "!*.md" | grep -i -E "image|icon|memory|performance"
Length of output: 2649
|
Don't think we need to change doc. Nobody is using this I think. |
Kinda sad. Kinda truth |
|
This use to work before we added http support and async. Glad to see it back. Thank you. |
|
🥷 Code experts: no user matched threshold 10 See detailsTo learn more about /:\ gitStream - Visit our Docs |
Also, bit of silly sanitizing data (accept only
data:image)Any documentation changes needed?