Skip to content

Commit

Permalink
Merge branch 'brindy/fireproofing-fix' into hotfix/7.105.1-changes
Browse files Browse the repository at this point in the history
  • Loading branch information
brindy committed Jan 24, 2024
2 parents c3ee1ca + 2df585d commit 3e75d0e
Show file tree
Hide file tree
Showing 5 changed files with 302 additions and 84 deletions.
86 changes: 78 additions & 8 deletions Core/CookieStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,27 @@ import Foundation

public class CookieStorage {

struct Constants {
static let key = "com.duckduckgo.allowedCookies"
struct Keys {
static let allowedCookies = "com.duckduckgo.allowedCookies"
static let consumed = "com.duckduckgo.consumedCookies"
}

private var userDefaults: UserDefaults


var isConsumed: Bool {
get {
userDefaults.bool(forKey: Keys.consumed, defaultValue: false)
}
set {
userDefaults.set(newValue, forKey: Keys.consumed)
}
}

/// Use the `updateCookies` function rather than the setter which is only visible for testing.
var cookies: [HTTPCookie] {
get {
var storedCookies = [HTTPCookie]()
if let cookies = userDefaults.object(forKey: Constants.key) as? [[String: Any?]] {
if let cookies = userDefaults.object(forKey: Keys.allowedCookies) as? [[String: Any?]] {
for cookieData in cookies {
var properties = [HTTPCookiePropertyKey: Any]()
cookieData.forEach({
Expand All @@ -57,17 +68,76 @@ public class CookieStorage {
}
cookies.append(mappedCookie)
}
userDefaults.setValue(cookies, forKey: Constants.key)
userDefaults.setValue(cookies, forKey: Keys.allowedCookies)
}

}

public init(userDefaults: UserDefaults = UserDefaults.app) {
self.userDefaults = userDefaults
}

func clear() {
userDefaults.removeObject(forKey: Constants.key)
os_log("cleared cookies", log: .generalLog, type: .debug)
enum CookieDomainsOnUpdate {
case empty
case match
case missing
case different
}

@discardableResult
func updateCookies(_ cookies: [HTTPCookie], keepingPreservedLogins preservedLogins: PreserveLogins) -> CookieDomainsOnUpdate {
isConsumed = false

let persisted = self.cookies

func cookiesByDomain(_ cookies: [HTTPCookie]) -> [String: [HTTPCookie]] {
var byDomain = [String: [HTTPCookie]]()
cookies.forEach { cookie in
var cookies = byDomain[cookie.domain, default: []]
cookies.append(cookie)
byDomain[cookie.domain] = cookies
}
return byDomain
}

let updatedCookiesByDomain = cookiesByDomain(cookies)
var persistedCookiesByDomain = cookiesByDomain(persisted)

// Do the diagnostics before the dicts get changed.
let diagnosticResult = evaluateDomains(
updatedDomains: updatedCookiesByDomain.keys.sorted(),
persistedDomains: persistedCookiesByDomain.keys.sorted()
)

updatedCookiesByDomain.keys.forEach {
persistedCookiesByDomain[$0] = updatedCookiesByDomain[$0]
}

persistedCookiesByDomain.keys.forEach {
guard $0 != "duckduckgo.com" else { return } // DDG cookies are for SERP settings only

if !preservedLogins.isAllowed(cookieDomain: $0) {
persistedCookiesByDomain.removeValue(forKey: $0)
}
}

let now = Date()
self.cookies = persistedCookiesByDomain.map { $0.value }.joined().compactMap { $0 }
.filter { $0.expiresDate == nil || $0.expiresDate! > now }

return diagnosticResult
}

private func evaluateDomains(updatedDomains: [String], persistedDomains: [String]) -> CookieDomainsOnUpdate {
if persistedDomains.isEmpty {
return .empty
} else if updatedDomains.count < persistedDomains.count {
return .missing
} else if updatedDomains == persistedDomains {
return .match
} else {
return .different
}
}

}
15 changes: 6 additions & 9 deletions Core/WebCacheManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public class WebCacheManager {

let cookies = cookieStorage.cookies

guard !cookies.isEmpty else {
guard !cookies.isEmpty, !cookieStorage.isConsumed else {
completion()
return
}
Expand All @@ -93,9 +93,9 @@ public class WebCacheManager {

DispatchQueue.global(qos: .userInitiated).async {
group.wait()
cookieStorage.isConsumed = true

DispatchQueue.main.async {
cookieStorage.clear()
completion()

if cookieStorage.cookies.count > 0 {
Expand Down Expand Up @@ -162,7 +162,7 @@ public class WebCacheManager {
logins: PreserveLogins,
storeIdManager: DataStoreIdManager,
completion: @escaping () -> Void) {

guard let containerId = storeIdManager.id else {
completion()
return
Expand All @@ -181,11 +181,8 @@ public class WebCacheManager {
await checkForLeftBehindDataStores()

storeIdManager.allocateNewContainerId()
// If cookies is empty it's likely that the webview was not used since the last fire button so
// don't overwrite previously saved cookies
if let cookies, !cookies.isEmpty {
cookieStorage.cookies = cookies
}

cookieStorage.updateCookies(cookies ?? [], keepingPreservedLogins: logins)

completion()
}
Expand All @@ -208,7 +205,7 @@ public class WebCacheManager {
// From this point onwards... use containers
dataStoreIdManager.allocateNewContainerId()
Task { @MainActor in
cookieStorage.cookies = cookies
cookieStorage.updateCookies(cookies, keepingPreservedLogins: logins)
completion()
}
} else {
Expand Down
4 changes: 4 additions & 0 deletions DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@
85A1B3B220C6CD9900C18F15 /* CookieStorage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85A1B3B120C6CD9900C18F15 /* CookieStorage.swift */; };
85A313972028E78A00327D00 /* release_notes.txt in Resources */ = {isa = PBXBuildFile; fileRef = 85A313962028E78A00327D00 /* release_notes.txt */; };
85A9C37920E0E00C00073340 /* HomeRow.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 85A9C37820E0E00C00073340 /* HomeRow.xcassets */; };
85AD49EE2B6149110085D2D1 /* CookieStorageTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85AD49ED2B6149110085D2D1 /* CookieStorageTests.swift */; };
85AE668E2097206E0014CF04 /* NotificationView.xib in Resources */ = {isa = PBXBuildFile; fileRef = 85AE668D2097206E0014CF04 /* NotificationView.xib */; };
85AE6690209724120014CF04 /* NotificationView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85AE668F209724120014CF04 /* NotificationView.swift */; };
85AFA1212B45D14F0028A504 /* BookmarksMigrationAssertionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85AFA1202B45D14F0028A504 /* BookmarksMigrationAssertionTests.swift */; };
Expand Down Expand Up @@ -1534,6 +1535,7 @@
85A313962028E78A00327D00 /* release_notes.txt */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; name = release_notes.txt; path = fastlane/metadata/default/release_notes.txt; sourceTree = "<group>"; };
85A53EC9200D1FA20010D13F /* FileStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FileStore.swift; sourceTree = "<group>"; };
85A9C37820E0E00C00073340 /* HomeRow.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = HomeRow.xcassets; sourceTree = "<group>"; };
85AD49ED2B6149110085D2D1 /* CookieStorageTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CookieStorageTests.swift; sourceTree = "<group>"; };
85AE668D2097206E0014CF04 /* NotificationView.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = NotificationView.xib; sourceTree = "<group>"; };
85AE668F209724120014CF04 /* NotificationView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationView.swift; sourceTree = "<group>"; };
85AFA1202B45D14F0028A504 /* BookmarksMigrationAssertionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksMigrationAssertionTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -5257,6 +5259,7 @@
8540BD5123D8C2220057FDD2 /* PreserveLoginsTests.swift */,
850559D123CF710C0055C0D5 /* WebCacheManagerTests.swift */,
F198D7971E3A45D90088DA8A /* WKWebViewConfigurationExtensionTests.swift */,
85AD49ED2B6149110085D2D1 /* CookieStorageTests.swift */,
);
name = Web;
sourceTree = "<group>";
Expand Down Expand Up @@ -6991,6 +6994,7 @@
8521FDE6238D414B00A44CC3 /* FileStoreTests.swift in Sources */,
F14E491F1E391CE900DC037C /* URLExtensionTests.swift in Sources */,
85D2187424BF25CD004373D2 /* FaviconsTests.swift in Sources */,
85AD49EE2B6149110085D2D1 /* CookieStorageTests.swift in Sources */,
CBCCF96828885DEE006F4A71 /* AppPrivacyConfigurationTests.swift in Sources */,
310742AB2848E6FD0012660B /* BackForwardMenuHistoryItemURLSanitizerTests.swift in Sources */,
22CB1ED8203DDD2C00D2C724 /* AppDeepLinksTests.swift in Sources */,
Expand Down
188 changes: 188 additions & 0 deletions DuckDuckGoTests/CookieStorageTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
//
// CookieStorageTests.swift
// DuckDuckGo
//
// Copyright © 2024 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import XCTest
@testable import Core
import WebKit

public class CookieStorageTests: XCTestCase {

var storage: CookieStorage!

// This is updated by the `make` function which preserves any cookies added as part of this test
let logins = PreserveLogins.shared

static let userDefaultsSuiteName = "test"

public override func setUp() {
super.setUp()
let defaults = UserDefaults(suiteName: Self.userDefaultsSuiteName)!
defaults.removePersistentDomain(forName: Self.userDefaultsSuiteName)
storage = CookieStorage(userDefaults: defaults)
logins.clearAll()
}

func testWhenUpdatedThenDuckDuckGoCookiesAreNotRemoved() {
storage.updateCookies([
make("duckduckgo.com", name: "x", value: "1"),
], keepingPreservedLogins: logins)

XCTAssertEqual(1, storage.cookies.count)

storage.updateCookies([
make("test.com", name: "x", value: "1"),
], keepingPreservedLogins: logins)

XCTAssertEqual(2, storage.cookies.count)

}

func testWhenUpdatedThenCookiesWithFutureExpirationAreNotRemoved() {
storage.updateCookies([
make("test.com", name: "x", value: "1", expires: .distantFuture),
], keepingPreservedLogins: logins)

storage.updateCookies([
make("example.com", name: "x", value: "1"),
], keepingPreservedLogins: logins)

XCTAssertEqual(2, storage.cookies.count)
XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "test.com" }))
XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "example.com" }))

}

func testWhenUpdatingThenExistingExpiredCookiesAreRemoved() {
storage.cookies = [
make("test.com", name: "x", value: "1", expires: Date(timeIntervalSinceNow: -100)),
]
XCTAssertEqual(1, storage.cookies.count)

storage.updateCookies([
make("example.com", name: "x", value: "1"),
], keepingPreservedLogins: logins)

XCTAssertEqual(1, storage.cookies.count)
XCTAssertFalse(storage.cookies.contains(where: { $0.domain == "test.com" }))
XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "example.com" }))

}

func testWhenExpiredCookieIsAddedThenItIsNotPersisted() {

storage.updateCookies([
make("example.com", name: "x", value: "1", expires: Date(timeIntervalSinceNow: -100)),
], keepingPreservedLogins: logins)

XCTAssertEqual(0, storage.cookies.count)

}

func testWhenUpdatedThenNoLongerPreservedDomainsAreCleared() {
storage.updateCookies([
make("test.com", name: "x", value: "1"),
make("example.com", name: "x", value: "1"),
], keepingPreservedLogins: logins)

logins.remove(domain: "test.com")

storage.updateCookies([
make("example.com", name: "x", value: "1"),
], keepingPreservedLogins: logins)

XCTAssertEqual(1, storage.cookies.count)
XCTAssertFalse(storage.cookies.contains(where: { $0.domain == "test.com" }))
XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "example.com" }))
}

func testWhenStorageInitialiedThenItIsEmptyAndConsumedIsFalse() {
XCTAssertEqual(0, storage.cookies.count)
XCTAssertEqual(false, storage.isConsumed)
}

func testWhenStorageIsUpdatedThenConsumedIsResetToFalse() {
storage.isConsumed = true
XCTAssertTrue(storage.isConsumed)
storage.updateCookies([
make("test.com", name: "x", value: "1")
], keepingPreservedLogins: logins)
XCTAssertFalse(storage.isConsumed)
}

func testWhenStorageIsReinstanciatedThenUsesStoredData() {
storage.updateCookies([
make("test.com", name: "x", value: "1")
], keepingPreservedLogins: logins)
storage.isConsumed = true

let otherStorage = CookieStorage(userDefaults: UserDefaults(suiteName: Self.userDefaultsSuiteName)!)
XCTAssertEqual(1, otherStorage.cookies.count)
XCTAssertTrue(otherStorage.isConsumed)
}

func testWhenStorageIsUpdatedThenUpdatingAddsNewCookies() {
storage.updateCookies([
make("test.com", name: "x", value: "1")
], keepingPreservedLogins: logins)
XCTAssertEqual(1, storage.cookies.count)
}

func testWhenStorageIsUpdatedThenExistingCookiesAreUnaffected() {
storage.updateCookies([
make("test.com", name: "x", value: "1"),
make("example.com", name: "x", value: "1"),
], keepingPreservedLogins: logins)

storage.updateCookies([
make("example.com", name: "x", value: "2"),
], keepingPreservedLogins: logins)

XCTAssertEqual(2, storage.cookies.count)
XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "test.com" && $0.name == "x" && $0.value == "1" }))
XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "example.com" && $0.name == "x" && $0.value == "2" }))
}

func testWhenStorageHasMatchingDOmainThenUpdatingReplacesCookies() {
storage.updateCookies([
make("test.com", name: "x", value: "1")
], keepingPreservedLogins: logins)

storage.updateCookies([
make("test.com", name: "x", value: "2"),
make("test.com", name: "y", value: "3"),
], keepingPreservedLogins: logins)

XCTAssertEqual(2, storage.cookies.count)
XCTAssertFalse(storage.cookies.contains(where: { $0.domain == "test.com" && $0.name == "x" && $0.value == "1" }))
XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "test.com" && $0.name == "x" && $0.value == "2" }))
XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "test.com" && $0.name == "y" && $0.value == "3" }))
}

func make(_ domain: String, name: String, value: String, expires: Date? = nil) -> HTTPCookie {
logins.addToAllowed(domain: domain)
return HTTPCookie(properties: [
.domain: domain,
.name: name,
.value: value,
.path: "/",
.expires: expires as Any
])!
}

}
Loading

0 comments on commit 3e75d0e

Please sign in to comment.