From aed36ae6f7442575258679f5a9fc5a43be03b1da Mon Sep 17 00:00:00 2001
From: Fernando Bunn <bunn@idevzilla.com>
Date: Wed, 15 Feb 2023 16:22:13 +0000
Subject: [PATCH] Fix issue with %2C being sent instead of comma for broken
 site report (#1489)

Task/Issue URL: https://app.asana.com/0/414709148257752/1203055727448001/f
Tech Design URL:
CC:

Description:
Fix issue with %2C when sending a broken site report request
---
 Core/APIRequest.swift                          | 5 ++++-
 Core/Pixel.swift                               | 7 ++++++-
 DuckDuckGo/BrokenSiteInfo.swift                | 8 ++++++--
 DuckDuckGoTests/BrokenSiteReportingTests.swift | 7 ++-----
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/Core/APIRequest.swift b/Core/APIRequest.swift
index b97b1d737f..cc501a9e1d 100644
--- a/Core/APIRequest.swift
+++ b/Core/APIRequest.swift
@@ -102,6 +102,7 @@ public class APIRequest {
     public static func request<C: Collection>(url: URL,
                                               method: HTTPMethod = .get,
                                               parameters: C,
+                                              allowedQueryReservedCharacters: CharacterSet? = nil,
                                               headers: HTTPHeaders = APIHeaders().defaultHeaders,
                                               httpBody: Data? = nil,
                                               timeoutInterval: TimeInterval = 60.0,
@@ -114,6 +115,7 @@ public class APIRequest {
         let urlRequest = urlRequestFor(url: url,
                                        method: method,
                                        parameters: parameters,
+                                       allowedQueryReservedCharacters: allowedQueryReservedCharacters,
                                        headers: headers,
                                        httpBody: httpBody,
                                        timeoutInterval: timeoutInterval)
@@ -163,12 +165,13 @@ public class APIRequest {
     private static func urlRequestFor<C: Collection>(url: URL,
                                                      method: HTTPMethod,
                                                      parameters: C,
+                                                     allowedQueryReservedCharacters: CharacterSet? = nil,
                                                      headers: HTTPHeaders,
                                                      httpBody: Data?,
                                                      timeoutInterval: TimeInterval) -> URLRequest
     where C.Element == (key: String, value: String) {
 
-        let url = url.appendingParameters(parameters)
+        let url = url.appendingParameters(parameters, allowedReservedCharacters: allowedQueryReservedCharacters)
         var urlRequest = URLRequest.developerInitiated(url)
         urlRequest.allHTTPHeaderFields = headers
         urlRequest.httpMethod = method.rawValue
diff --git a/Core/Pixel.swift b/Core/Pixel.swift
index 09e0d46ede..b6003b7809 100644
--- a/Core/Pixel.swift
+++ b/Core/Pixel.swift
@@ -138,6 +138,7 @@ public class Pixel {
     public static func fire(pixel: Pixel.Event,
                             forDeviceType deviceType: UIUserInterfaceIdiom? = UIDevice.current.userInterfaceIdiom,
                             withAdditionalParameters params: [String: String] = [:],
+                            allowedQueryReservedCharacters: CharacterSet? = nil,
                             withHeaders headers: HTTPHeaders = APIHeaders().defaultHeaders,
                             includedParameters: [QueryParameters] = [.atb, .appVersion],
                             onComplete: @escaping (Error?) -> Void = { _ in }) {
@@ -163,7 +164,11 @@ public class Pixel {
             url = appUrls.pixelUrl(forPixelNamed: pixel.name, includeATB: includedParameters.contains(.atb) )
         }
         
-        APIRequest.request(url: url, parameters: newParams, headers: headers, callBackOnMainThread: true) { (_, error) in
+        APIRequest.request(url: url,
+                           parameters: newParams,
+                           allowedQueryReservedCharacters: allowedQueryReservedCharacters,
+                           headers: headers,
+                           callBackOnMainThread: true) { (_, error) in
             
             os_log("Pixel fired %s %s", log: .generalLog, type: .debug, pixel.name, "\(params)")
             onComplete(error)
diff --git a/DuckDuckGo/BrokenSiteInfo.swift b/DuckDuckGo/BrokenSiteInfo.swift
index 5ad0dd2ba8..3d403b4bbf 100644
--- a/DuckDuckGo/BrokenSiteInfo.swift
+++ b/DuckDuckGo/BrokenSiteInfo.swift
@@ -21,7 +21,9 @@ import Foundation
 import Core
 
 public struct BrokenSiteInfo {
-    
+
+    static let allowedQueryReservedCharacters =  CharacterSet(charactersIn: ",")
+
     private struct Keys {
         static let url = "siteUrl"
         static let category = "category"
@@ -98,7 +100,9 @@ public struct BrokenSiteInfo {
                           Keys.ampUrl: ampUrl ?? "",
                           Keys.urlParametersRemoved: urlParametersRemoved ? "true" : "false"]
         
-        Pixel.fire(pixel: .brokenSiteReport, withAdditionalParameters: parameters)
+        Pixel.fire(pixel: .brokenSiteReport,
+                   withAdditionalParameters: parameters,
+                   allowedQueryReservedCharacters: BrokenSiteInfo.allowedQueryReservedCharacters)
     }
     
     private func normalize(_ url: URL?) -> String {
diff --git a/DuckDuckGoTests/BrokenSiteReportingTests.swift b/DuckDuckGoTests/BrokenSiteReportingTests.swift
index 212bfaf5bc..4bf0ebecf8 100644
--- a/DuckDuckGoTests/BrokenSiteReportingTests.swift
+++ b/DuckDuckGoTests/BrokenSiteReportingTests.swift
@@ -80,9 +80,7 @@ final class BrokenSiteReportingTests: XCTestCase {
                                             systemVersion: test.os ?? "",
                                             gpc: test.gpcEnabled)
         
-        
-        let expectation = XCTestExpectation()
-        
+
         stub(condition: isHost(host)) { request -> HTTPStubsResponse in
             
             guard let requestURL = request.url else {
@@ -91,9 +89,8 @@ final class BrokenSiteReportingTests: XCTestCase {
             }
 
             let absoluteURL = requestURL.absoluteString
-                .replacingOccurrences(of: "%2C", with: ",")
                 .replacingOccurrences(of: "%20", with: " ")
-            
+
             if test.expectReportURLPrefix.count > 0 {
                 XCTAssertTrue(requestURL.absoluteString.contains(test.expectReportURLPrefix), "Prefix [\(test.expectReportURLPrefix)] not found")
             }