Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
Fix #3868: Do not store favicons of too big size. (#3874)
Browse files Browse the repository at this point in the history
Co-authored-by: Brandon-T <JustBrandonT@gmail.com>
  • Loading branch information
iccub and Brandon-T authored Jul 16, 2021
1 parent 8032cae commit c0f1a16
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 6 deletions.
7 changes: 7 additions & 0 deletions Client/Application/Migration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ class Migration {
if !Preferences.Migration.playlistV1FileSettingsLocationCompleted.value {
movePlaylistV1Items()
}

if !Preferences.Migration.removeLargeFaviconsMigrationCompleted.value {
FaviconMO.clearTooLargeFavicons()
Preferences.Migration.removeLargeFaviconsMigrationCompleted.value = true
}
}

static func moveDatabaseToApplicationDirectory() {
Expand Down Expand Up @@ -120,6 +125,8 @@ fileprivate extension Preferences {
Option<Bool>(key: "migration.documents-dir-completed", default: false)
static let playlistV1FileSettingsLocationCompleted =
Option<Bool>(key: "migration.playlistv1-file-settings-location-completed", default: false)
static let removeLargeFaviconsMigrationCompleted =
Option<Bool>(key: "migration.remove-large-favicons", default: false)
}

/// Migrate the users preferences from prior versions of the app (<2.0)
Expand Down
9 changes: 7 additions & 2 deletions Client/Frontend/Browser/BrowserTrayAnimators.swift
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,13 @@ private func createTransitionCellFromTab(_ tab: Tab?, withFrame frame: CGRect) -
cell.screenshotView.image = tab?.screenshot
cell.titleLabel.text = tab?.displayTitle

if let favIcon = tab?.displayFavicon {
cell.favicon.sd_setImage(with: URL(string: favIcon.url)!)
if let favIcon = tab?.displayFavicon, let url = URL(string: favIcon.url) {
let scale = UIScreen.main.scale
let thumbnailSize = CGSize(width: 50 * scale, height: 50 * scale)

cell.favicon.sd_setImage(with: url,
placeholderImage: #imageLiteral(resourceName: "defaultFavicon"),
context: [.imageThumbnailPixelSize: thumbnailSize])
} else {
cell.favicon.image = #imageLiteral(resourceName: "defaultFavicon")
}
Expand Down
22 changes: 21 additions & 1 deletion Client/Frontend/Browser/FaviconHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,26 @@ class FaviconHandler {
let onCompletedSiteFavicon: ImageCacheCompletion = { image, data, _, _, url in
let favicon = Favicon(url: url.absoluteString, date: Date(), type: type)

guard let image = image else {
guard let image = image,
let imageData = data else {
favicon.width = 0
favicon.height = 0

onSuccess(favicon, data)
return
}

if let header = "%PDF".data(using: .utf8),
imageData.count >= header.count,
let range = imageData.range(of: header),
range.lowerBound.distance(to: imageData.startIndex) < 8 { //strict PDF parsing. Otherwise index <= (1024 - header.count)
// ^8 is the best range because some PDF's can contain a UTF-8 BOM (Byte-Order Mark)

favicon.width = 0
favicon.height = 0
onSuccess(favicon, data)
return
}

favicon.width = Int(image.size.width)
favicon.height = Int(image.size.height)
Expand Down Expand Up @@ -99,6 +112,13 @@ extension FaviconHandler: TabEventHandler {
TabEvent.post(.didLoadFavicon(favicon, with: data), for: tab)
}
}
// No favicon fetched from metadata, trying base domain's standard favicon location.
else if let baseURL = tab.url?.domainURL {
loadFaviconURL(baseURL.appendingPathComponent("favicon.ico").absoluteString,
type: .icon, forTab: tab) >>== { (favicon, data) in
TabEvent.post(.didLoadFavicon(favicon, with: data), for: tab)
}
}
}
}

Expand Down
21 changes: 18 additions & 3 deletions Client/Frontend/Browser/ImageCache/WebImageCache.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@

import Foundation
import SDWebImage
import Data
import Shared
import BraveShared

private let log = Logger.browserLogger

final class WebImageCache: ImageCacheProtocol {

Expand Down Expand Up @@ -51,11 +56,21 @@ final class WebImageCache: ImageCacheProtocol {
if options.contains(.lowPriority) {
webImageOptions.append(.lowPriority)
}


webImageOptions.append(.scaleDownLargeImages)

let imageOperation = webImageManager.loadImage(with: url, options: SDWebImageOptions(webImageOptions), context: webImageContext, progress: progressBlock) { image, data, error, webImageCacheType, _, imageURL in
let key = self.webImageManager.cacheKey(for: url)
if let image = image, !self.isPrivate {
self.webImageManager.imageCache.store(image, imageData: data, forKey: key, cacheType: .all)
if let image = image {
let maxSize = CGFloat(FaviconMO.maxSize)
if image.size.width > maxSize || image.size.height > maxSize {
log.warning("Favicon size too big, not storing it in cache.")
return
}

if !self.isPrivate {
self.webImageManager.imageCache.store(image, imageData: data, forKey: key, cacheType: .all)
}
}

let cacheType = self.mapImageCacheType(from: webImageCacheType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const {getMetadata:metadataparser, metadataRuleSets} = require("page-metadata-pa
function MetadataWrapper() {
this.getMetadata = function() {
const customRuleSets = metadataRuleSets;
customRuleSets.icon.defaultValue = () => "";
customRuleSets.icon.rules = [
['link[rel="icon" i]', element => element.getAttribute('href')],
['link[rel="fluid-icon"]', element => element.getAttribute('href')],
Expand Down
22 changes: 22 additions & 0 deletions Data/models/FaviconMO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import UIKit
import CoreData
import Foundation
import Storage
import Shared
import BraveShared

private let log = Logger.browserLogger

public final class FaviconMO: NSManagedObject, CRUD {

Expand All @@ -12,10 +16,23 @@ public final class FaviconMO: NSManagedObject, CRUD {
@NSManaged public var height: Int32
@NSManaged public var type: Int16
@NSManaged public var domain: Domain?

/// Maximum size of favicon that can be stored on the device.
public static let maxSize = 1024

// MARK: - Public interface

public class func add(_ favicon: Favicon, forSiteUrl siteUrl: URL, persistent: Bool) {
guard let width = favicon.width, let height = favicon.height else {
log.warning("Failed to unwrap favicon's width or height")
return
}

if width > maxSize || height > maxSize {
log.warning("Favicon to save is too large.")
return
}

DataController.perform(context: .new(inMemory: !persistent)) { context in
var item = FaviconMO.get(forFaviconUrl: favicon.url, context: context)
if item == nil {
Expand Down Expand Up @@ -46,6 +63,11 @@ public final class FaviconMO: NSManagedObject, CRUD {
}
}
}

public static func clearTooLargeFavicons() {
let predicate = NSPredicate(format: "width > \(maxSize) OR height > \(maxSize)")
deleteAll(predicate: predicate)
}
}

// MARK: - Internal implementations
Expand Down

0 comments on commit c0f1a16

Please sign in to comment.