From dc2a72adf712726fda9d8ef0a90b993d56f4fad7 Mon Sep 17 00:00:00 2001 From: Leendert de Borst Date: Sat, 13 Dec 2025 18:14:58 +0100 Subject: [PATCH] Add favicon extraction reuse logic (#1404) --- .../popup/pages/credentials/ItemAddEdit.tsx | 62 ++---- .../src/utils/FaviconService.ts | 210 ++++++++++++++++++ .../src/utils/SqliteClient.ts | 58 +++-- 3 files changed, 269 insertions(+), 61 deletions(-) create mode 100644 apps/browser-extension/src/utils/FaviconService.ts diff --git a/apps/browser-extension/src/entrypoints/popup/pages/credentials/ItemAddEdit.tsx b/apps/browser-extension/src/entrypoints/popup/pages/credentials/ItemAddEdit.tsx index e3af86129..45d481310 100644 --- a/apps/browser-extension/src/entrypoints/popup/pages/credentials/ItemAddEdit.tsx +++ b/apps/browser-extension/src/entrypoints/popup/pages/credentials/ItemAddEdit.tsx @@ -1,5 +1,3 @@ -import { Buffer } from 'buffer'; - import React, { useState, useEffect, useCallback, useMemo, useRef } from 'react'; import { useTranslation } from 'react-i18next'; import { useNavigate, useParams, useSearchParams } from 'react-router-dom'; @@ -31,8 +29,8 @@ import { useVaultMutate } from '@/entrypoints/popup/hooks/useVaultMutate'; import { SKIP_FORM_RESTORE_KEY } from '@/utils/Constants'; import type { Item, ItemField, ItemType, FieldType, Attachment, TotpCode } from '@/utils/dist/core/models/vault'; import { FieldCategories, FieldTypes, ItemTypes, getSystemFieldsForItemType, isFieldShownByDefault } from '@/utils/dist/core/models/vault'; +import { FaviconService } from '@/utils/FaviconService'; import { ServiceDetectionUtility } from '@/utils/serviceDetection/ServiceDetectionUtility'; -import { SqliteClient } from '@/utils/SqliteClient'; import { browser } from '#imports'; @@ -338,8 +336,10 @@ const ItemAddEdit: React.FC = () => { ? itemTypeParam : DEFAULT_ITEM_TYPE; - // Get URL parameters for service detection (e.g., from content script popout) - // Use searchParams from react-router which handles hash-based routing correctly + /* + * Get URL parameters for service detection (e.g., from content script popout) + * Use searchParams from react-router which handles hash-based routing correctly + */ const serviceNameFromUrl = searchParams.get('serviceName'); const serviceUrlFromUrl = searchParams.get('serviceUrl'); const currentUrl = searchParams.get('currentUrl'); @@ -665,53 +665,21 @@ const ItemAddEdit: React.FC = () => { } }); - const updatedItem: Item = { + let updatedItem: Item = { ...item, Fields: fields, UpdatedAt: new Date().toISOString() }; - // Extract favicon from URL if the item has one and no logo exists for this source - const urlValue = fieldValues['login.url']; - const urlList = Array.isArray(urlValue) ? urlValue : urlValue ? [urlValue] : []; - // Find the first valid URL (starts with http://, https://, or www.) - const validUrl = urlList.find(url => { - const trimmed = url?.trim(); - return trimmed && (trimmed.startsWith('http://') || trimmed.startsWith('https://') || trimmed.startsWith('www.')); - }); - // Normalize URL: prepend https:// if it starts with www. - const urlString = validUrl?.startsWith('www.') ? `https://${validUrl}` : validUrl; - - if (urlString && dbContext?.sqliteClient) { - // Extract and normalize the source domain from the URL - const source = SqliteClient.extractSourceFromUrl(urlString); - - // Check if a logo already exists for this source - if (dbContext.sqliteClient.hasLogoForSource(source)) { - console.debug(`[Favicon] Logo already exists for source "${source}", skipping fetch`); - } else { - console.debug(`[Favicon] No logo found for source "${source}", fetching...`); - setLocalLoading(true); - try { - const timeoutPromise = new Promise((_, reject) => - setTimeout(() => reject(new Error('Favicon extraction timed out')), 5000) - ); - - const faviconPromise = webApi.get<{ image: string }>('Favicon/Extract?url=' + urlString); - const faviconResponse = await Promise.race([faviconPromise, timeoutPromise]) as { image: string }; - - console.debug('[Favicon] Response received:', faviconResponse?.image ? 'has image' : 'no image'); - - if (faviconResponse?.image) { - const decodedImage = Uint8Array.from(Buffer.from(faviconResponse.image, 'base64')); - updatedItem.Logo = decodedImage; - console.debug('[Favicon] Logo decoded and attached to item'); - } - } catch (err) { - // Favicon extraction failed or timed out, this is not a critical error so we can ignore it. - console.error('[Favicon] Error extracting favicon:', err); - } - } + // Fetch and attach favicon from URL if needed (handles deduplication internally) + if (dbContext?.sqliteClient) { + setLocalLoading(true); + updatedItem = await FaviconService.fetchAndAttachFavicon( + updatedItem, + fieldValues['login.url'], + dbContext.sqliteClient, + webApi + ); } // Save to database and sync vault diff --git a/apps/browser-extension/src/utils/FaviconService.ts b/apps/browser-extension/src/utils/FaviconService.ts new file mode 100644 index 000000000..9ee774d55 --- /dev/null +++ b/apps/browser-extension/src/utils/FaviconService.ts @@ -0,0 +1,210 @@ +import { Buffer } from 'buffer'; + +import type { Item } from '@/utils/dist/core/models/vault'; +import type { SqliteClient } from '@/utils/SqliteClient'; +import type { WebApiService } from '@/utils/WebApiService'; + +/** + * Result of a favicon fetch operation. + */ +export type FaviconFetchResult = { + /** Whether a favicon was successfully fetched */ + success: boolean; + /** The decoded favicon image data (if successful) */ + imageData?: Uint8Array; + /** Whether the fetch was skipped because a logo already exists */ + skipped?: boolean; + /** Error message (if failed) */ + error?: string; +}; + +/** + * Default timeout for favicon fetch operations (5 seconds). + */ +const FAVICON_FETCH_TIMEOUT_MS = 5000; + +/** + * Centralized service for favicon/logo operations. + * Handles URL normalization, deduplication, and favicon fetching. + */ +export class FaviconService { + /** + * Extract and normalize source domain from a URL string. + * This matches the server-side migration logic for consistent deduplication. + * Uses lowercase and removes www. prefix for case-insensitive matching. + * @param urlString The URL to extract the domain from + * @returns The normalized source domain (e.g., 'github.com'), or 'unknown' if extraction fails + */ + public static extractSourceFromUrl(urlString: string | undefined | null): string { + if (!urlString) { + return 'unknown'; + } + + try { + const url = new URL(urlString.startsWith('http') ? urlString : `https://${urlString}`); + // Normalize hostname: lowercase and remove www. prefix + return url.hostname.toLowerCase().replace(/^www\./, ''); + } catch { + return 'unknown'; + } + } + + /** + * Normalize a URL string for favicon fetching. + * Prepends https:// if the URL starts with www. + * @param url The URL to normalize + * @returns The normalized URL, or undefined if invalid + */ + public static normalizeUrl(url: string | undefined | null): string | undefined { + if (!url) { + return undefined; + } + + const trimmed = url.trim(); + if (!trimmed) { + return undefined; + } + + // Check if it's a valid URL format + if (!trimmed.startsWith('http://') && !trimmed.startsWith('https://') && !trimmed.startsWith('www.')) { + return undefined; + } + + // Prepend https:// if starts with www. + return trimmed.startsWith('www.') ? `https://${trimmed}` : trimmed; + } + + /** + * Extract the first valid URL from a field value (which can be string or string[]). + * @param urlValue The URL field value + * @returns The first valid URL, or undefined if none found + */ + public static extractFirstValidUrl(urlValue: string | string[] | undefined | null): string | undefined { + if (!urlValue) { + return undefined; + } + + const urlList = Array.isArray(urlValue) ? urlValue : [urlValue]; + + // Find the first valid URL (starts with http://, https://, or www.) + const validUrl = urlList.find(url => { + const trimmed = url?.trim(); + return trimmed && (trimmed.startsWith('http://') || trimmed.startsWith('https://') || trimmed.startsWith('www.')); + }); + + return FaviconService.normalizeUrl(validUrl); + } + + /** + * Check if a logo already exists for the given URL. + * @param urlString The URL to check + * @param sqliteClient The SQLite client instance + * @returns True if a logo exists for the normalized source domain + */ + public static hasLogoForUrl(urlString: string | undefined | null, sqliteClient: SqliteClient): boolean { + if (!urlString) { + return false; + } + + const source = FaviconService.extractSourceFromUrl(urlString); + if (source === 'unknown') { + return false; + } + + return sqliteClient.hasLogoForSource(source); + } + + /** + * Fetch favicon for a URL from the server API. + * Includes deduplication check and timeout handling. + * @param urlString The URL to fetch favicon for + * @param sqliteClient The SQLite client for deduplication check + * @param webApi The WebAPI service for making the request + * @param timeoutMs Optional timeout in milliseconds (default: 5000ms) + * @returns FaviconFetchResult with success status and image data + */ + public static async fetchFavicon( + urlString: string | undefined | null, + sqliteClient: SqliteClient, + webApi: WebApiService, + timeoutMs: number = FAVICON_FETCH_TIMEOUT_MS + ): Promise { + // Validate URL + const normalizedUrl = FaviconService.normalizeUrl(urlString); + if (!normalizedUrl) { + return { success: false, error: 'Invalid URL' }; + } + + // Extract source for deduplication check + const source = FaviconService.extractSourceFromUrl(normalizedUrl); + if (source === 'unknown') { + return { success: false, error: 'Could not extract domain from URL' }; + } + + // Check if logo already exists (deduplication) + if (sqliteClient.hasLogoForSource(source)) { + console.debug(`[Favicon] Logo already exists for source "${source}", skipping fetch`); + return { success: false, skipped: true }; + } + + console.debug(`[Favicon] No logo found for source "${source}", fetching...`); + + try { + // Create timeout promise + const timeoutPromise = new Promise((_, reject) => + setTimeout(() => reject(new Error('Favicon extraction timed out')), timeoutMs) + ); + + // Fetch favicon from API + const faviconPromise = webApi.get<{ image: string }>(`Favicon/Extract?url=${encodeURIComponent(normalizedUrl)}`); + const faviconResponse = await Promise.race([faviconPromise, timeoutPromise]); + + console.debug('[Favicon] Response received:', faviconResponse?.image ? 'has image' : 'no image'); + + if (faviconResponse?.image) { + const decodedImage = Uint8Array.from(Buffer.from(faviconResponse.image, 'base64')); + console.debug('[Favicon] Logo decoded successfully'); + return { success: true, imageData: decodedImage }; + } + + return { success: false, error: 'No favicon returned from server' }; + } catch (err) { + // Favicon extraction failed or timed out - not critical + const errorMessage = err instanceof Error ? err.message : 'Unknown error'; + console.error('[Favicon] Error extracting favicon:', errorMessage); + return { success: false, error: errorMessage }; + } + } + + /** + * Fetch and attach favicon to an item if needed. + * This is a convenience method that combines URL extraction, deduplication, and fetching. + * @param item The item to potentially update with a logo + * @param urlFieldValue The value of the URL field (can be string or string[]) + * @param sqliteClient The SQLite client for deduplication check + * @param webApi The WebAPI service for making the request + * @returns The updated item with Logo attached (if favicon was fetched), or the original item + */ + public static async fetchAndAttachFavicon( + item: Item, + urlFieldValue: string | string[] | undefined | null, + sqliteClient: SqliteClient, + webApi: WebApiService + ): Promise { + const urlString = FaviconService.extractFirstValidUrl(urlFieldValue); + if (!urlString) { + return item; + } + + const result = await FaviconService.fetchFavicon(urlString, sqliteClient, webApi); + + if (result.success && result.imageData) { + return { + ...item, + Logo: result.imageData + }; + } + + return item; + } +} diff --git a/apps/browser-extension/src/utils/SqliteClient.ts b/apps/browser-extension/src/utils/SqliteClient.ts index fa4a89fdc..eafb470bf 100644 --- a/apps/browser-extension/src/utils/SqliteClient.ts +++ b/apps/browser-extension/src/utils/SqliteClient.ts @@ -2155,18 +2155,21 @@ export class SqliteClient { const currentDateTime = dateFormatter.now(); const itemId = item.Id || crypto.randomUUID().toUpperCase(); - // 1. Handle Logo - get or create logo entry + // 1. Handle Logo - get or create logo entry, or link to existing logo by source let logoId: string | null = null; + const urlField = item.Fields?.find(f => f.FieldKey === 'login.url'); + const urlValue = urlField?.Value; + const urlString = Array.isArray(urlValue) ? urlValue[0] : urlValue; + const source = SqliteClient.extractSourceFromUrl(urlString); + if (item.Logo) { const logoData = SqliteClient.convertLogoToUint8Array(item.Logo); if (logoData) { - // Extract source from URL field - const urlField = item.Fields?.find(f => f.FieldKey === 'login.url'); - const urlValue = urlField?.Value; - const urlString = Array.isArray(urlValue) ? urlValue[0] : urlValue; - const source = SqliteClient.extractSourceFromUrl(urlString); logoId = this.getOrCreateLogoId(source, logoData, currentDateTime); } + } else if (source !== 'unknown') { + // No new logo provided, but check if an existing logo exists for this source + logoId = this.getLogoIdForSource(source); } // 2. Insert Item @@ -2333,21 +2336,27 @@ export class SqliteClient { const currentDateTime = dateFormatter.now(); - // 1. Handle Logo - get or create logo entry + // 1. Handle Logo - get or create logo entry, or link to existing logo by source let logoId: string | null = null; + const urlField = item.Fields?.find(f => f.FieldKey === 'login.url'); + const urlValue = urlField?.Value; + const urlString = Array.isArray(urlValue) ? urlValue[0] : urlValue; + const source = SqliteClient.extractSourceFromUrl(urlString); + if (item.Logo) { const logoData = SqliteClient.convertLogoToUint8Array(item.Logo); if (logoData) { - // Extract source from URL field - const urlField = item.Fields?.find(f => f.FieldKey === 'login.url'); - const urlValue = urlField?.Value; - const urlString = Array.isArray(urlValue) ? urlValue[0] : urlValue; - const source = SqliteClient.extractSourceFromUrl(urlString); logoId = this.getOrCreateLogoId(source, logoData, currentDateTime); } + } else if (source !== 'unknown') { + /* + * No new logo provided, but check if an existing logo exists for this source. + * This handles the case where URL was changed to a domain we already have a logo for. + */ + logoId = this.getLogoIdForSource(source); } - // 2. Update Item (including LogoId if a new logo was provided) + // 2. Update Item (including LogoId if a new logo was provided or existing one found) const itemQuery = ` UPDATE Items SET Name = ?, @@ -2845,6 +2854,7 @@ export class SqliteClient { /** * Permanently delete an item - converts to tombstone for sync. * Hard deletes all child entities and marks item as IsDeleted=1. + * Also cleans up orphaned logos (logos no longer referenced by any item). * @param itemId - The ID of the item to permanently delete * @returns The number of rows updated */ @@ -2858,6 +2868,11 @@ export class SqliteClient { const currentDateTime = dateFormatter.now(); + // 0. Get the LogoId before we clear it (for orphan cleanup) + const logoQuery = `SELECT LogoId FROM Items WHERE Id = ?`; + const logoResult = this.executeQuery<{ LogoId: string | null }>(logoQuery, [itemId]); + const logoId = logoResult.length > 0 ? logoResult[0].LogoId : null; + // 1. Hard delete all FieldValues for this item this.executeUpdate(`DELETE FROM FieldValues WHERE ItemId = ?`, [itemId]); @@ -2870,7 +2885,7 @@ export class SqliteClient { // 4. Hard delete all TotpCodes for this item this.executeUpdate(`DELETE FROM TotpCodes WHERE ItemId = ?`, [itemId]); - // 5. Hard delete all Attachments for this item + // 5. Hard delete all Attachments for this item (including blob data) this.executeUpdate(`DELETE FROM Attachments WHERE ItemId = ?`, [itemId]); // 6. Hard delete all ItemTags for this item @@ -2888,6 +2903,21 @@ export class SqliteClient { const result = this.executeUpdate(itemQuery, [currentDateTime, itemId]); + // 8. Clean up orphaned logo if this was the last item using it + if (logoId) { + const logoUsageQuery = ` + SELECT COUNT(*) as count FROM Items + WHERE LogoId = ? AND IsDeleted = 0`; + const usageResult = this.executeQuery<{ count: number }>(logoUsageQuery, [logoId]); + const usageCount = usageResult.length > 0 ? usageResult[0].count : 0; + + if (usageCount === 0) { + // No other items reference this logo, hard delete it + this.executeUpdate(`DELETE FROM Logos WHERE Id = ?`, [logoId]); + console.debug(`[SqliteClient] Deleted orphaned logo: ${logoId}`); + } + } + await this.commitTransaction(); return result; } catch (error) {