Skip to content

Commit

Permalink
Fix issue with %2C being sent instead of comma for broken site report (
Browse files Browse the repository at this point in the history
…duckduckgo#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
  • Loading branch information
Bunn authored Feb 15, 2023
1 parent 40f434e commit aed36ae
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 9 deletions.
5 changes: 4 additions & 1 deletion Core/APIRequest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -114,6 +115,7 @@ public class APIRequest {
let urlRequest = urlRequestFor(url: url,
method: method,
parameters: parameters,
allowedQueryReservedCharacters: allowedQueryReservedCharacters,
headers: headers,
httpBody: httpBody,
timeoutInterval: timeoutInterval)
Expand Down Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion Core/Pixel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) {
Expand All @@ -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)
Expand Down
8 changes: 6 additions & 2 deletions DuckDuckGo/BrokenSiteInfo.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
7 changes: 2 additions & 5 deletions DuckDuckGoTests/BrokenSiteReportingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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")
}
Expand Down

0 comments on commit aed36ae

Please sign in to comment.