From 88e4b71fe3b5c6bc602ac503231d26508a6c627d Mon Sep 17 00:00:00 2001 From: Lily Vulcano Date: Thu, 28 Feb 2019 13:03:15 -0800 Subject: [PATCH] Fix PropertyListSerialization not doing a roundtrip of Data correctly. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In 4.x, bridging `Data` to an object produces a `_NSSwiftData`, a `NSData` subclass. `CFData…` functions weren’t set up for it, so now they must be. Unfortunately, due to the way the implementation of s-c-f differs from Darwin’s — since we have no class clusters in Swift — there’s some wrangling needed in NSMutableData to prevent infinite recursion when its base class’s methods are invoked. --- .../Base.subproj/ForSwiftFoundationOnly.h | 16 +++++++ CoreFoundation/Collections.subproj/CFData.c | 47 ++++++++++++++++-- Foundation/NSData.swift | 31 +++++++++++- Foundation/NSSwiftRuntime.swift | 5 ++ .../TestPropertyListSerialization.swift | 48 +++++++++++++++---- 5 files changed, 134 insertions(+), 13 deletions(-) diff --git a/CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h b/CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h index 85b0b88715..2f9a7ce0b3 100644 --- a/CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h +++ b/CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h @@ -239,6 +239,13 @@ struct _NSNumberBridge { bool (*_Nonnull _getValue)(CFTypeRef number, void *value, CFNumberType type); }; +struct _NSDataBridge { + _Nonnull CFTypeRef (*_Nonnull copy)(CFTypeRef obj); + CFIndex (*_Nonnull length)(CFTypeRef obj); + const void *_Nonnull (*_Nonnull bytePtr)(CFTypeRef obj); + void (*_Nonnull getBytes)(CFTypeRef obj, CFRange range, void *_Nonnull buffer); +}; + struct _CFSwiftBridge { struct _NSObjectBridge NSObject; struct _NSArrayBridge NSArray; @@ -254,6 +261,7 @@ struct _CFSwiftBridge { struct _NSCharacterSetBridge NSCharacterSet; struct _NSMutableCharacterSetBridge NSMutableCharacterSet; struct _NSNumberBridge NSNumber; + struct _NSDataBridge NSData; }; CF_EXPORT struct _CFSwiftBridge __CFSwiftBridge; @@ -423,6 +431,14 @@ static inline unsigned int _dev_minor(dev_t rdev) { return minor(rdev); } +#pragma mark - Data Functions + +CF_CROSS_PLATFORM_EXPORT CFDataRef _CFDataCreateCopyNoBridging(CFAllocatorRef allocator, CFDataRef data); +CF_CROSS_PLATFORM_EXPORT const uint8_t *_CFDataGetBytePtrNoBridging(CFDataRef data); +CF_CROSS_PLATFORM_EXPORT uint8_t *_CFDataGetMutableBytePtrNoBridging(CFDataRef data); +CF_CROSS_PLATFORM_EXPORT void _CFDataGetBytesNoBridging(CFDataRef data, CFRange range, void *buffer); +CF_CROSS_PLATFORM_EXPORT CFIndex _CFDataGetLengthNoBridging(CFDataRef data); + _CF_EXPORT_SCOPE_END #endif /* __COREFOUNDATION_FORSWIFTFOUNDATIONONLY__ */ diff --git a/CoreFoundation/Collections.subproj/CFData.c b/CoreFoundation/Collections.subproj/CFData.c index 54620d3f27..9554bd067e 100644 --- a/CoreFoundation/Collections.subproj/CFData.c +++ b/CoreFoundation/Collections.subproj/CFData.c @@ -456,11 +456,13 @@ CFDataRef CFDataCreateWithBytesNoCopy(CFAllocatorRef allocator, const uint8_t *b return __CFDataInit(allocator, kCFImmutable, length, bytes, length, bytesDeallocator); } -CFDataRef CFDataCreateCopy(CFAllocatorRef allocator, CFDataRef data) { +static CFDataRef _CFDataCreateCopyAllowingBridging(CFAllocatorRef allocator, CFDataRef data, Boolean allowBridging) { Boolean allowRetain = true; if (allowRetain) { - CF_OBJC_FUNCDISPATCHV(CFDataGetTypeID(), CFDataRef, (NSData *)data, copy); - CF_SWIFT_FUNCDISPATCHV(CFDataGetTypeID(), CFDataRef, (CFSwiftRef)data, NSObject.copyWithZone, NULL); + if (allowBridging) { + CF_OBJC_FUNCDISPATCHV(CFDataGetTypeID(), CFDataRef, (NSData *)data, copy); + CF_SWIFT_FUNCDISPATCHV(CFDataGetTypeID(), CFDataRef, (CFSwiftRef)data, NSData.copy); + } // If the data isn't mutable... if (!__CFDataIsMutable(data)) { @@ -485,6 +487,16 @@ CFDataRef CFDataCreateCopy(CFAllocatorRef allocator, CFDataRef data) { return __CFDataInit(allocator, kCFImmutable, length, CFDataGetBytePtr(data), length, NULL); } +CFDataRef CFDataCreateCopy(CFAllocatorRef allocator, CFDataRef data) { + return _CFDataCreateCopyAllowingBridging(allocator, data, true); +} + +#if DEPLOYMENT_RUNTIME_SWIFT +CF_CROSS_PLATFORM_EXPORT CFDataRef _CFDataCreateCopyNoBridging(CFAllocatorRef allocator, CFDataRef data) { + return _CFDataCreateCopyAllowingBridging(allocator, data, false); +} +#endif + CF_PRIVATE CFMutableDataRef _CFDataCreateFixedMutableWithBuffer(CFAllocatorRef allocator, CFIndex capacity, const uint8_t *bytes, CFAllocatorRef bytesDeallocator) { return (CFMutableDataRef)__CFDataInit(allocator, kCFFixedMutable, capacity, bytes, 0, bytesDeallocator); } @@ -501,9 +513,17 @@ CFMutableDataRef CFDataCreateMutableCopy(CFAllocatorRef allocator, CFIndex capac CFIndex CFDataGetLength(CFDataRef data) { CF_OBJC_FUNCDISPATCHV(CFDataGetTypeID(), CFIndex, (NSData *)data, length); + CF_SWIFT_FUNCDISPATCHV(CFDataGetTypeID(), CFIndex, (CFSwiftRef)data, NSData.length); + __CFGenericValidateType(data, CFDataGetTypeID()); + return __CFDataLength(data); +} + +#if DEPLOYMENT_RUNTIME_SWIFT +CF_CROSS_PLATFORM_EXPORT CFIndex _CFDataGetLengthNoBridging(CFDataRef data) { __CFGenericValidateType(data, CFDataGetTypeID()); return __CFDataLength(data); } +#endif CF_PRIVATE uint8_t *_CFDataGetBytePtrNonObjC(CFDataRef data) { __CFGenericValidateType(data, CFDataGetTypeID()); @@ -512,21 +532,42 @@ CF_PRIVATE uint8_t *_CFDataGetBytePtrNonObjC(CFDataRef data) { const uint8_t *CFDataGetBytePtr(CFDataRef data) { CF_OBJC_FUNCDISPATCHV(CFDataGetTypeID(), const uint8_t *, (NSData *)data, bytes); + CF_SWIFT_FUNCDISPATCHV(CFDataGetTypeID(), const uint8_t *, (CFSwiftRef)data, NSData.bytePtr); return _CFDataGetBytePtrNonObjC(data); } +#if DEPLOYMENT_RUNTIME_SWIFT +CF_CROSS_PLATFORM_EXPORT const uint8_t *_CFDataGetBytePtrNoBridging(CFDataRef data) { + return _CFDataGetBytePtrNonObjC(data); +} +#endif + uint8_t *CFDataGetMutableBytePtr(CFMutableDataRef data) { CF_OBJC_FUNCDISPATCHV(CFDataGetTypeID(), uint8_t *, (NSMutableData *)data, mutableBytes); CFAssert1(__CFDataIsMutable(data), __kCFLogAssertion, "%s(): data is immutable", __PRETTY_FUNCTION__); return _CFDataGetBytePtrNonObjC(data); } +#if DEPLOYMENT_RUNTIME_SWIFT +CF_CROSS_PLATFORM_EXPORT uint8_t *_CFDataGetMutableBytePtrNoBridging(CFDataRef data) { + return _CFDataGetBytePtrNonObjC(data); +} +#endif + void CFDataGetBytes(CFDataRef data, CFRange range, uint8_t *buffer) { CF_OBJC_FUNCDISPATCHV(CFDataGetTypeID(), void, (NSData *)data, getBytes:(void *)buffer range:NSMakeRange(range.location, range.length)); + CF_SWIFT_FUNCDISPATCHV(CFDataGetTypeID(), void, (CFSwiftRef)data, NSData.getBytes, range, buffer); __CFDataValidateRange(data, range, __PRETTY_FUNCTION__); memmove(buffer, _CFDataGetBytePtrNonObjC(data) + range.location, range.length); } +#if DEPLOYMENT_RUNTIME_SWIFT +CF_CROSS_PLATFORM_EXPORT void _CFDataGetBytesNoBridging(CFDataRef data, CFRange range, void *buffer) { + __CFDataValidateRange(data, range, __PRETTY_FUNCTION__); + memmove(buffer, _CFDataGetBytePtrNonObjC(data) + range.location, range.length); +} +#endif + /* Allocates new block of data with at least numNewValues more bytes than the current length. If clear is true, the new bytes up to at least the new length with be zeroed. */ static void __CFDataGrow(CFMutableDataRef data, CFIndex numNewValues, Boolean clear) { CFIndex oldLength = __CFDataLength(data); diff --git a/Foundation/NSData.swift b/Foundation/NSData.swift index 12a865abe6..52f2012058 100644 --- a/Foundation/NSData.swift +++ b/Foundation/NSData.swift @@ -977,17 +977,28 @@ open class NSMutableData : NSData { super.init(base64Encoded: base64Data, options: options) } + open override var bytes: UnsafeRawPointer { + return UnsafeRawPointer(_CFDataGetBytePtrNoBridging(_cfObject)) + } + + open override func copy() -> Any { + return _CFDataCreateCopyNoBridging(kCFAllocatorSystemDefault, _cfObject)._nsObject + } // MARK: - Funnel Methods /// A pointer to the data contained by the mutable data object. open var mutableBytes: UnsafeMutableRawPointer { - return UnsafeMutableRawPointer(CFDataGetMutableBytePtr(_cfMutableObject)) + return UnsafeMutableRawPointer(_CFDataGetMutableBytePtrNoBridging(_cfMutableObject)) + } + + open override func getBytes(_ buffer: UnsafeMutableRawPointer, range: NSRange) { + return _CFDataGetBytesNoBridging(_cfObject, CFRange(range), buffer) } /// The number of bytes contained in the mutable data object. open override var length: Int { get { - return CFDataGetLength(_cfObject) + return _CFDataGetLengthNoBridging(_cfObject) } set { CFDataSetLength(_cfMutableObject, newValue) @@ -1064,3 +1075,19 @@ extension NSData : _StructTypeBridgeable { return Data._unconditionallyBridgeFromObjectiveC(self) } } + +internal func _CFSwiftDataCreateCopy(_ data: AnyObject) -> Unmanaged { + return Unmanaged.passRetained((data as! NSData).copy() as! NSObject) +} + +internal func _CFSwiftDataGetLength(_ data: AnyObject) -> CFIndex { + return CFIndex((data as! NSData).length) +} + +internal func _CFSwiftDataGetBytePtr(_ data: AnyObject) -> UnsafeRawPointer { + return (data as! NSData).bytes +} + +internal func _CFSwiftDataGetBytes(_ data: AnyObject, _ range: CFRange, _ buffer: UnsafeMutableRawPointer) { + (data as! NSData).getBytes(buffer, range: NSRange(range)) +} diff --git a/Foundation/NSSwiftRuntime.swift b/Foundation/NSSwiftRuntime.swift index eb0fb1a7dc..37c1102acf 100644 --- a/Foundation/NSSwiftRuntime.swift +++ b/Foundation/NSSwiftRuntime.swift @@ -286,6 +286,11 @@ internal func __CFInitializeSwift() { __CFSwiftBridge.NSNumber._getValue = _CFSwiftNumberGetValue __CFSwiftBridge.NSNumber.boolValue = _CFSwiftNumberGetBoolValue + __CFSwiftBridge.NSData.copy = _CFSwiftDataCreateCopy + __CFSwiftBridge.NSData.bytePtr = _CFSwiftDataGetBytePtr + __CFSwiftBridge.NSData.length = _CFSwiftDataGetLength + __CFSwiftBridge.NSData.getBytes = _CFSwiftDataGetBytes + // __CFDefaultEightBitStringEncoding = UInt32(kCFStringEncodingUTF8) } diff --git a/TestFoundation/TestPropertyListSerialization.swift b/TestFoundation/TestPropertyListSerialization.swift index 8b02661f0c..952b0e49ac 100644 --- a/TestFoundation/TestPropertyListSerialization.swift +++ b/TestFoundation/TestPropertyListSerialization.swift @@ -8,14 +8,6 @@ // class TestPropertyListSerialization : XCTestCase { - static var allTests: [(String, (TestPropertyListSerialization) -> () throws -> Void)] { - return [ - ("test_BasicConstruction", test_BasicConstruction), - ("test_decodeData", test_decodeData), - ("test_decodeStream", test_decodeStream), - ] - } - func test_BasicConstruction() { let dict = NSMutableDictionary(capacity: 0) // dict["foo"] = "bar" @@ -79,4 +71,44 @@ class TestPropertyListSerialization : XCTestCase { XCTFail("value stored is not a string") } } + + func test_roundtripBasicTypes() throws { + let name: String = "my name" + let age: Int = 100 + let data: Data = "helloworld".data(using: .utf8)! + + let properties: [String: Any] = [ + "name": name, + "age": age, + "data": data + ] + + // pack + let serialized = try PropertyListSerialization.data(fromPropertyList: properties, format: .binary, options: 0) + + // unpack + let deserialized = try PropertyListSerialization.propertyList(from: serialized, options: [], format: nil) + let dictionary = try (deserialized as? [String: Any]).unwrapped() + + XCTAssertNotNil(dictionary["name"]) + XCTAssertNotNil(dictionary["name"] as? String) + XCTAssertEqual(name, dictionary["name"] as? String) + + XCTAssertNotNil(dictionary["age"]) + XCTAssertNotNil(dictionary["age"] as? Int) + XCTAssertEqual(age, dictionary["age"] as? Int) + + XCTAssertNotNil(dictionary["data"]) + XCTAssertNotNil(dictionary["data"] as? Data) + XCTAssertEqual(data, dictionary["data"] as? Data) + } + + static var allTests: [(String, (TestPropertyListSerialization) -> () throws -> Void)] { + return [ + ("test_BasicConstruction", test_BasicConstruction), + ("test_decodeData", test_decodeData), + ("test_decodeStream", test_decodeStream), + ("test_roundtripBasicTypes", test_roundtripBasicTypes), + ] + } }