Fix potential crash in #177 (#178)

* Switched OutputFileStream to use FileHandle

* updated Storage for OutputFileStream changes

* Added unableToClose handler

* Fixed test issue w/ parallelized tests.

* Removed dead code

* Flush memory contents to file before closing.

* Fix lack of version callouts.

* Added version checks for sync/close

Author: bsneed

Sources/Segment/Errors.swift

@@ -12,6 +12,7 @@ public enum AnalyticsError: Error {
     case storageUnableToWrite(String)
     case storageUnableToRename(String)
     case storageUnableToOpen(String)
+    case storageUnableToClose(String)
     case storageInvalid(String)
     case storageUnknown(Error)
 
@@ -41,6 +42,8 @@ extension Analytics {
                 return AnalyticsError.storageUnableToOpen(path)
             case .unableToWrite(let path):
                 return AnalyticsError.storageUnableToWrite(path)
+            case .unableToClose(let path):
+                return AnalyticsError.storageUnableToClose(path)
             }
         }
 

Sources/Segment/Utilities/OutputFileStream.swift

@@ -21,9 +21,10 @@ internal class OutputFileStream {
         case unableToOpen(String)
         case unableToWrite(String)
         case unableToCreate(String)
+        case unableToClose(String)
     }
 
-    var filePointer: UnsafeMutablePointer<FILE>? = nil
+    var fileHandle: FileHandle? = nil
     let fileURL: URL
 
     init(fileURL: URL) throws {
@@ -49,36 +50,49 @@ internal class OutputFileStream {
 
     /// Open simply opens the file, no attempt at creation is made.
     func open() throws {
-        if filePointer != nil { return }
-        let path = fileURL.path
-        path.withCString { file in
-            filePointer = fopen(file, "w")
+        if fileHandle != nil { return }
+        do {
+            fileHandle = try FileHandle(forWritingTo: fileURL)
+        } catch {
+            throw OutputStreamError.unableToOpen(fileURL.path)
         }
-        guard filePointer != nil else { throw OutputStreamError.unableToOpen(path) }
     }
 
     func write(_ data: Data) throws {
-        guard let string = String(data: data, encoding: .utf8) else { return }
-        try write(string)
+        guard data.isEmpty == false else { return }
+        if #available(macOS 10.15.4, iOS 13.4, macCatalyst 13.4, tvOS 13.4, watchOS 13.4, *) {
+            do {
+                try fileHandle?.write(contentsOf: data)
+            } catch {
+                throw OutputStreamError.unableToWrite(fileURL.path)
+            }
+        } else {
+            // Fallback on earlier versions
+            fileHandle?.write(data)
+        }
     }
 
     func write(_ string: String) throws {
         guard string.isEmpty == false else { return }
-        _ = try string.utf8.withContiguousStorageIfAvailable { str in
-            if let baseAddr = str.baseAddress {
-                fwrite(baseAddr, 1, str.count, filePointer)
-            } else {
-                throw OutputStreamError.unableToWrite(fileURL.path)
-            }
-            if ferror(filePointer) != 0 {
-                throw OutputStreamError.unableToWrite(fileURL.path)
-            }
+        if let data = string.data(using: .utf8) {
+            try write(data)
         }
     }
 
-    func close() {
-        fclose(filePointer)
-        filePointer = nil
+    func close() throws {
+        do {
+            let existing = fileHandle
+            fileHandle = nil
+            if #available(tvOS 13.0, *) {
+                try existing?.synchronize() // this might be overkill, but JIC.
+                try existing?.close()
+            } else {
+                // Fallback on earlier versions
+                existing?.synchronizeFile()
+                existing?.closeFile()
+            }
+        } catch {
+            throw OutputStreamError.unableToClose(fileURL.path)
+        }
     }
 }
-

Sources/Segment/Utilities/Storage.swift

@@ -321,10 +321,11 @@ extension Storage {
         let fileEnding = "],\"sentAt\":\"\(sentAt)\",\"writeKey\":\"\(writeKey)\"}"
         do {
             try outputStream.write(fileEnding)
+            try outputStream.close()
         } catch {
             analytics?.reportInternalError(error)
         }
-        outputStream.close()
+        
         self.outputStream = nil
 
         let tempFile = file.appendingPathExtension(Storage.tempExtension)

Tests/Segment-Tests/Analytics_Tests.swift

@@ -533,16 +533,23 @@ final class Analytics_Tests: XCTestCase {
     }
 
     func testRequestFactory() {
-        let config = Configuration(writeKey: "test").requestFactory { request in
+        let config = Configuration(writeKey: "testSequential").requestFactory { request in
             XCTAssertEqual(request.value(forHTTPHeaderField: "Accept-Encoding"), "gzip")
             XCTAssertEqual(request.value(forHTTPHeaderField: "Content-Type"), "application/json; charset=utf-8")
-            XCTAssertEqual(request.value(forHTTPHeaderField: "Authorization"), "Basic test")
+            XCTAssertEqual(request.value(forHTTPHeaderField: "Authorization"), "Basic testSequential")
             XCTAssertTrue(request.value(forHTTPHeaderField: "User-Agent")!.contains("analytics-ios/"))
             return request
         }.errorHandler { error in
-            XCTFail("\(error)")
+            switch error {
+            case AnalyticsError.networkServerRejected(_):
+                // we expect this one; it's a bogus writekey
+                break;
+            default:
+                XCTFail("\(error)")
+            }
         }
         let analytics = Analytics(configuration: config)
+        analytics.storage.hardReset(doYouKnowHowToUseThis: true)
         let outputReader = OutputReaderPlugin()
         analytics.add(plugin: outputReader)