NSString bridging and internal NUL, take two [SR-2225]

Foundation.xcodeproj/project.pbxproj

@@ -1272,6 +1272,7 @@
 				61E0117B1C1B554D000037DD /* TestNSRunLoop.swift */,
 				844DC3321C17584F005611F9 /* TestNSScanner.swift */,
 				EA66F6411BF1619600136161 /* TestNSSet.swift */,
+				0383A1741D2E558A0052E5D1 /* TestNSStream.swift */,
 				EA66F6421BF1619600136161 /* TestNSString.swift */,
 				5B6F17941C48631C00935030 /* TestNSTask.swift */,
 				5FE52C941D147D1C00F7D270 /* TestNSTextCheckingResult.swift */,
@@ -1289,7 +1290,6 @@
 				5B40F9F11C125187000E72E3 /* TestNSXMLParser.swift */,
 				CC5249BF1D341D23007CB54D /* TestUnitConverter.swift */,
 				5B6F17961C48631C00935030 /* TestUtils.swift */,
-				0383A1741D2E558A0052E5D1 /* TestNSStream.swift */,
 			);
 			name = Tests;
 			sourceTree = "<group>";

Foundation/NSString.swift

@@ -86,19 +86,56 @@ extension String : _ObjectTypeBridgeable {
             result = x._storage
         } else if type(of: x) == _NSCFString.self {
             let cf = unsafeBitCast(x, to: CFString.self)
-            let str = CFStringGetCStringPtr(cf, CFStringEncoding(kCFStringEncodingUTF8))
-            if str != nil {
-                result = String(cString: str!)
-            } else {
-                let length = CFStringGetLength(cf)
-                let buffer = UnsafeMutablePointer<UniChar>.allocate(capacity: length)
-                CFStringGetCharacters(cf, CFRangeMake(0, length), buffer)
-
-                let str = String._fromCodeUnitSequence(UTF16.self, input: UnsafeBufferPointer(start: buffer, count: length))
-                buffer.deinitialize(count: length)
-                buffer.deallocate(capacity: length)
-                result = str
-            }
+
+            // An ideal fast path would use the result of calling:
+            //
+            // ```
+            // CFStringGetCStringPtr(cf, CFStringEncoding(kCFStringEncodingUTF8)
+            // ```
+            //
+            // ...but, subsequently calling `String(cString:)` blows away any
+            // text that comes after an internal null character (which is
+            // correct behavior on the part of the initializer but not what we
+            // want here).
+            //
+            // The next best thing for a fast path would be to call, instead of
+            // `String(cString:)`, `String._fromCodeUnitSequence(_:input:)`.
+            // But, to do so we would want an `UnsafeBufferPointer`, for which
+            // we'd need to know the length in bytes of our string.
+            //
+            // As it happens, `CFStringGetLength(_:)` seems to give the length
+            // *in bytes* if `CFStringGetCStringPtr(_:_:)` doesn't return nil.
+            // But what the documentation claims is that `CFStringGetLength(_:)`
+            // should return the length *in UTF-16 code pairs*. We shouldn't
+            // rely on buggy behavior.
+            //
+            // We're left with doing things the slow way, which involves first
+            // allocating a buffer to hold some data.
+            //
+            // One method (the original) uses `CFStringGetCharacters(_:_:_:)` to
+            // write Unicode character data to that buffer, but as it turns out,
+            // like `CFStringGetLength(_:)`, `CFStringGetCharacters(_:_:_:)`
+            // doesn't do what it should with a `CFString` created with bytes or
+            // a `CFString` created with a C string *if* multi-byte characters
+            // are involved.
+            //
+            // So even our slow path needs some work.
+
+            // FIXME: CF functions don't behave as documented; hijinx ensue.
+            //        If those issues (discussed above) are resolved, we can do
+            //        much better than this.
+
+            let length = CFStringGetLength(cf) // This value is utterly unreliable
+
+            // Retrieving Unicode characters is unreliable; instead retrieve UTF8-encoded bytes
+            let max = CFStringGetMaximumSizeForEncoding(length, kCFStringEncodingUTF8)
+            let buffer = UnsafeMutablePointer<UInt8>.allocate(capacity: max)
+            var count: CFIndex = -1
+            CFStringGetBytes(cf, CFRangeMake(0, length), kCFStringEncodingUTF8, 0, false, buffer, max, &count)
+            let str = String._fromCodeUnitSequence(UTF8.self, input: UnsafeBufferPointer(start: buffer, count: count))
+            buffer.deinitialize(count: length)
+            buffer.deallocate(capacity: length)
+            result = str
         } else if type(of: x) == _NSCFConstantString.self {
             let conststr = unsafeBitCast(x, to: _NSCFConstantString.self)
             let str = String._fromCodeUnitSequence(UTF8.self, input: UnsafeBufferPointer(start: conststr._ptr, count: Int(conststr._length)))
@@ -1221,9 +1258,7 @@ extension NSString {
     }
 
     public convenience init?(data: Data, encoding: UInt) {
-        guard let cf = data.withUnsafeBytes({ (bytes: UnsafePointer<UInt8>) -> CFString? in
-            return CFStringCreateWithBytes(kCFAllocatorDefault, bytes, data.count, CFStringConvertNSStringEncodingToEncoding(encoding), true)
-        }) else { return nil }
+        guard let cf = CFStringCreateFromExternalRepresentation(kCFAllocatorDefault, data._cfObject, CFStringConvertNSStringEncodingToEncoding(encoding)) else { return nil }
 
         var str: String?
         if String._conditionallyBridgeFromObject(cf._nsObject, result: &str) {

TestFoundation/TestNSJSONSerialization.swift

@@ -592,7 +592,7 @@ extension TestNSJSONSerialization {
             ("test_jsonObjectToOutputStreamBuffer", test_jsonObjectToOutputStreamBuffer),
             ("test_jsonObjectToOutputStreamFile", test_jsonObjectToOutputStreamFile),
             ("test_invalidJsonObjectToStreamBuffer", test_invalidJsonObjectToStreamBuffer),
-            ("test_jsonObjectToOutputStreamInsufficeintBuffer", test_jsonObjectToOutputStreamInsufficeintBuffer),
+            ("test_jsonObjectToOutputStreamInsufficientBuffer", test_jsonObjectToOutputStreamInsufficientBuffer),
         ]
     }
 
@@ -777,7 +777,7 @@ extension TestNSJSONSerialization {
             let result = try JSONSerialization.writeJSONObject(dict.bridge(), toStream: outputStream, options: [])
             outputStream.close()
             if(result > -1) {
-                XCTAssertEqual(NSString(bytes: buffer, length: buffer.count, encoding: String.Encoding.utf8.rawValue), "{\"a\":{\"b\":1}}")
+                XCTAssertEqual(NSString(bytes: buffer, length: buffer.count, encoding: String.Encoding.utf8.rawValue), "{\"a\":{\"b\":1}}\0\0\0\0\0\0\0")
             }
         } catch {
             XCTFail("Error thrown: \(error)")
@@ -801,7 +801,7 @@ extension TestNSJSONSerialization {
                         let resultRead: Int = fileStream.read(&buffer, maxLength: buffer.count)
                         fileStream.close()
                         if(resultRead > -1){
-                            XCTAssertEqual(NSString(bytes: buffer, length: buffer.count, encoding: String.Encoding.utf8.rawValue), "{\"a\":{\"b\":1}}")
+                            XCTAssertEqual(NSString(bytes: buffer, length: buffer.count, encoding: String.Encoding.utf8.rawValue), "{\"a\":{\"b\":1}}\0\0\0\0\0\0\0")
                         }
                     }
                     removeTestFile(filePath!)
@@ -814,7 +814,7 @@ extension TestNSJSONSerialization {
         }
     }
 
-    func test_jsonObjectToOutputStreamInsufficeintBuffer() {
+    func test_jsonObjectToOutputStreamInsufficientBuffer() {
         let dict = ["a":["b":1]]
         let buffer = Array<UInt8>(repeating: 0, count: 10)
         let outputStream = NSOutputStream(toBuffer: UnsafeMutablePointer(mutating: buffer), capacity: 20)

TestFoundation/TestNSStream.swift

@@ -33,7 +33,7 @@ class TestNSStream : XCTestCase {
     }
 
     func test_InputStreamWithData(){
-        let message: NSString = "Hello, playground"
+        let message: NSString = "Hello, playground\0\0\0"
         let messageData: Data = message.data(using: String.Encoding.utf8.rawValue)!
         let dataStream: InputStream = InputStream(data: messageData)
         XCTAssertEqual(Stream.Status.notOpen, dataStream.streamStatus)
@@ -52,7 +52,7 @@ class TestNSStream : XCTestCase {
     }
 
     func test_InputStreamWithUrl() {
-        let message: NSString = "Hello, playground"
+        let message: NSString = "Hello, playground\0\0\0"
         let messageData: Data  = message.data(using: String.Encoding.utf8.rawValue)!
         //Initialiser with url
         let testFile = createTestFile("testFile_in.txt", _contents: messageData)
@@ -80,7 +80,7 @@ class TestNSStream : XCTestCase {
     }
 
     func test_InputStreamWithFile() {
-        let message: NSString = "Hello, playground"
+        let message: NSString = "Hello, playground\0\0\0"
         let messageData: Data  = message.data(using: String.Encoding.utf8.rawValue)!
         //Initialiser with file
         let testFile = createTestFile("testFile_in.txt", _contents: messageData)

TestFoundation/TestNSString.swift

@@ -59,6 +59,7 @@ class TestNSString : XCTestCase {
             ("test_FromNullTerminatedCStringInUTF8", test_FromNullTerminatedCStringInUTF8 ),
             // Swift3 updates broke the expectations of this test. disabling for now
             // ("test_FromMalformedNullTerminatedCStringInUTF8", test_FromMalformedNullTerminatedCStringInUTF8 ),
+            ("test_FromDataWithInternalNullCharacter", test_FromDataWithInternalNullCharacter ),
             ("test_uppercaseString", test_uppercaseString ),
             ("test_lowercaseString", test_lowercaseString ),
             ("test_capitalizedString", test_capitalizedString ),
@@ -69,7 +70,7 @@ class TestNSString : XCTestCase {
             ("test_FromContentOfFile",test_FromContentOfFile),
             ("test_swiftStringUTF16", test_swiftStringUTF16),
             // This test takes forever on build servers; it has been seen up to 1852.084 seconds
-//            ("test_completePathIntoString", test_completePathIntoString),
+            // ("test_completePathIntoString", test_completePathIntoString),
             ("test_stringByTrimmingCharactersInSet", test_stringByTrimmingCharactersInSet),
             ("test_initializeWithFormat", test_initializeWithFormat),
             ("test_initializeWithFormat2", test_initializeWithFormat2),
@@ -272,6 +273,14 @@ class TestNSString : XCTestCase {
         XCTAssertNil(string)
     }
 
+    func test_FromDataWithInternalNullCharacter() {
+        let bytes = mockASCIIStringBytes + [0x00] + mockASCIIStringBytes
+        let length = mockASCIIStringBytes.count * 2 + 1
+        let data = Data(bytes: bytes, count: length)
+        let string = NSString(data: data, encoding: String.Encoding.ascii.rawValue)
+        XCTAssertTrue(string?.isEqual(to: mockASCIIString + "\0" + mockASCIIString) ?? false)
+    }
+
     func test_FromContentsOfURL() {
         guard let testFileURL = testBundle().url(forResource: "NSStringTestData", withExtension: "txt") else {
             XCTFail("URL for NSStringTestData.txt is nil")