-
-
Notifications
You must be signed in to change notification settings - Fork 317
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: Crash in baggageEncodedDictionary (#4017)
The baggageEncodedDictionary method sometimes crashes with a null pointer exception in CFCharacterSetAddCharactersInString. It seems mutableCopy of NSCharacterSet.alphanumericCharacterSet or when calling stringByAddingPercentEncodingWithAllowedCharacters sometimes doesn't work. Instead of trying to fix the underlying issue of a null pointer exception, we can convert the code to Swift, designed for safety to avoid such mistakes. This should prevent the crash. Co-authored-by: Dhiogo Brustolin <dhiogo.brustolin@sentry.io>
- Loading branch information
1 parent
b0e218b
commit 77c9c47
Showing
9 changed files
with
124 additions
and
93 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import Foundation | ||
|
|
||
| @objcMembers | ||
| class SentryBaggageSerialization: NSObject { | ||
|
|
||
| private static let SENTRY_BAGGAGE_MAX_SIZE = 8_192 | ||
|
|
||
| static func encodeDictionary(_ dictionary: [String: String]) -> String { | ||
| var items: [String] = [] | ||
| items.reserveCapacity(dictionary.count) | ||
|
|
||
| var allowedSet = CharacterSet.alphanumerics | ||
| allowedSet.insert(charactersIn: "-_.") | ||
| var currentSize = 0 | ||
|
|
||
| for (key, value) in dictionary { | ||
| guard let keyDescription = key.addingPercentEncoding(withAllowedCharacters: allowedSet), | ||
| let valueDescription = value.addingPercentEncoding(withAllowedCharacters: allowedSet), !keyDescription.isEmpty && !valueDescription.isEmpty else { | ||
| continue | ||
| } | ||
|
|
||
| let item = "\(keyDescription)=\(valueDescription)" | ||
| if item.count + currentSize <= SENTRY_BAGGAGE_MAX_SIZE { | ||
| currentSize += item.count + 1 // +1 is to account for the comma that will be added for each extra item | ||
| items.append(item) | ||
| } | ||
| } | ||
|
|
||
| return items.sorted().joined(separator: ",") | ||
| } | ||
|
|
||
| static func decode(_ baggage: String) -> [String: String] { | ||
| guard !baggage.isEmpty else { | ||
| return [:] | ||
| } | ||
|
|
||
| var decoded: [String: String] = [:] | ||
|
|
||
| let properties = baggage.components(separatedBy: ",") | ||
|
|
||
| for property in properties { | ||
| let parts = property.components(separatedBy: "=") | ||
| guard parts.count == 2 else { | ||
| continue | ||
| } | ||
| let key = parts[0] | ||
| if let value = parts[1].removingPercentEncoding { | ||
| decoded[key] = value | ||
| } | ||
| } | ||
|
|
||
| return decoded | ||
| } | ||
| } |
57 changes: 57 additions & 0 deletions
57
Tests/SentryTests/Helper/SentryBaggageSerializationTests.swift
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| import Foundation | ||
| @testable import Sentry | ||
| import XCTest | ||
|
|
||
| class SentryBaggageSerializationTests: XCTestCase { | ||
|
|
||
| func testDictionaryToBaggageEncoded() { | ||
| XCTAssertEqual(encodeDictionary(["key": "value"]), "key=value") | ||
| XCTAssertEqual(encodeDictionary(["key": "value", "key2": "value2"]), "key2=value2,key=value") | ||
| XCTAssertEqual(encodeDictionary(["key": "value&"]), "key=value%26") | ||
| XCTAssertEqual(encodeDictionary(["key": "value="]), "key=value%3D") | ||
| XCTAssertEqual(encodeDictionary(["key": "value "]), "key=value%20") | ||
| XCTAssertEqual(encodeDictionary(["key": "value%"]), "key=value%25") | ||
| XCTAssertEqual(encodeDictionary(["key": "value-_"]), "key=value-_") | ||
| XCTAssertEqual(encodeDictionary(["key": "value\n\r"]), "key=value%0A%0D") | ||
|
|
||
| let largeValue = String(repeating: "a", count: 8_188) | ||
|
|
||
| XCTAssertEqual(encodeDictionary(["key": largeValue]), "key=\(largeValue)") | ||
| XCTAssertEqual(encodeDictionary(["AKey": "something", "BKey": largeValue]), "AKey=something") | ||
| XCTAssertEqual(encodeDictionary(["AKey": "something", "BKey": largeValue, "CKey": "Other Value"]), "AKey=something,CKey=Other%20Value") | ||
| } | ||
|
|
||
| func testBaggageEmptyKey_ReturnsEmptyString() { | ||
| XCTAssertEqual(encodeDictionary(["key": ""]), "") | ||
| } | ||
|
|
||
| func testBaggageEmptyValue_ReturnsEmptyString() { | ||
| XCTAssertEqual(encodeDictionary(["": "value"]), "") | ||
| } | ||
|
|
||
| func testBaggageEmptyKeyAndValue_ReturnsEmptyString() { | ||
| XCTAssertEqual(encodeDictionary(["": ""]), "") | ||
| } | ||
|
|
||
| func testBaggageStringToDictionaryDecoded() { | ||
| XCTAssertEqual(decode("key=value"), ["key": "value"]) | ||
| XCTAssertEqual(decode("key2=value2,key=value"), ["key": "value", "key2": "value2"]) | ||
| XCTAssertEqual(decode("key=value%26"), ["key": "value&"]) | ||
| XCTAssertEqual(decode("key=value%3D"), ["key": "value="]) | ||
| XCTAssertEqual(decode("key=value%20"), ["key": "value "]) | ||
| XCTAssertEqual(decode("key=value%25"), ["key": "value%"]) | ||
| XCTAssertEqual(decode("key=value-_"), ["key": "value-_"]) | ||
| XCTAssertEqual(decode("key=value%0A%0D"), ["key": "value\n\r"]) | ||
| XCTAssertEqual(decode(""), [:]) | ||
| XCTAssertEqual(decode("key"), [:]) | ||
| XCTAssertEqual(decode("key="), ["key": ""]) | ||
| } | ||
|
|
||
| private func encodeDictionary(_ dictionary: [String: String]) -> String { | ||
| return SentryBaggageSerialization.encodeDictionary(dictionary) | ||
| } | ||
|
|
||
| private func decode(_ baggage: String) -> [String: String] { | ||
| return SentryBaggageSerialization.decode(baggage) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters