GH-7: Fix hang when encountering an unexpected encoding.

Author: slythfox

Sources/Document.swift

@@ -3,7 +3,7 @@ import Foundation
 /**
     Convenience model for working with CSV in-memory.
 
-    - Note: Take advantage of data streaming capabilities when possible.
+    - Note: Take advantage of data streaming capabilities when possible instead of using this convenience class.
 */
 public class Document: InputHandlerDelegate {
 
@@ -61,6 +61,8 @@ public class Document: InputHandlerDelegate {
 
     /**
         Initialize a document from a CSV-formatted file.
+
+        - Note: Although this streams input data from the `FileHandle` the resulting document is still the full physical representation of the data.
     */
     public convenience init(fileHandle: FileHandle, dialect: Dialect = Dialect()) throws {
         self.init(dialect: dialect)

Sources/InputHandler.swift

@@ -39,6 +39,7 @@ public protocol InputHandlerDelegate: class {
 public class InputHandler {
 
     public static let defaultByteLength = 128
+    public static let defaultMaxRetries = 3
 
     /**
         Delegate responsible for handling events from the input stream.
@@ -53,29 +54,33 @@ public class InputHandler {
     */
     private(set) public var isOpen = false
     private var data = Data()
+    private let maxRetries: Int
+    private var retries: Int = 0
     private let fileHandle: FileHandle
     private var parser: ImportParser
 
     /**
         - Parameter fileHandle: FileHandle for reading. InputHandler should be solely responsible for controlling seeking behavior during its lifetime. The FileHandle's seek position should be at the beginning.
         - Parameter dialect: Dialect from which to parse against.
+        - Parameter maxRetries: Maximum number of allowed consecutive retries
     */
-    public init(fileHandle: FileHandle, dialect: Dialect = Dialect()) {
+    public init(fileHandle: FileHandle, dialect: Dialect = Dialect(), maxRetries: Int = InputHandler.defaultMaxRetries) {
         self.fileHandle = fileHandle
         self.dialect = dialect
+        self.maxRetries = maxRetries
         self.parser = ImportParser(dialect: dialect)
     }
 
-    public convenience init(from url: URL, dialect: Dialect = Dialect()) throws {
+    public convenience init(from url: URL, dialect: Dialect = Dialect(), maxRetries: Int = InputHandler.defaultMaxRetries) throws {
         let fileHandle = try FileHandle(forReadingFrom: url)
-        self.init(fileHandle: fileHandle, dialect: dialect)
+        self.init(fileHandle: fileHandle, dialect: dialect, maxRetries: maxRetries)
     }
 
-    public convenience init?(atPath path: String, dialect: Dialect = Dialect()) {
+    public convenience init?(atPath path: String, dialect: Dialect = Dialect(), maxRetries: Int = InputHandler.defaultMaxRetries) {
         guard let fileHandle = FileHandle(forReadingAtPath: path) else {
             return nil
         }
-        self.init(fileHandle: fileHandle, dialect: dialect)
+        self.init(fileHandle: fileHandle, dialect: dialect, maxRetries: maxRetries)
     }
 
     deinit {
@@ -85,8 +90,8 @@ public class InputHandler {
     /**
         Reads and parses a chunk of data from the FileHandle and directs output to its delegate.
 
-        - Parameter length: Maximum byte length of data to read.
-        - Return: Whether data was read. False indicates end of file.
+        - Parameter length: Maximum number of bytes of data to read.
+        - Returns: Whether data was read and to continue reading. Data may have been buffered. False indicates end of file.
         - Throws: May throw anything the `InputHandlerDelegate` or `ImportParser` throws. If thrown, state should be assumed to be invalid and should be reset before attempting to read again.
     */
     public func read(length: Int = InputHandler.defaultByteLength) throws -> Bool {
@@ -100,7 +105,13 @@ public class InputHandler {
         do {
             rows = try self.parser.import(data: data)
         } catch ImportParser.ImportError.badEncoding {
-            return true // We may have broken utf8 in receiving incomplete data
+            self.retries += 1
+            // We may have received incomplete data that broke UTF-8 decoding due to variable byte widths
+            // We give the reader an opportunity to read more before throwing that error
+            guard self.retries <= self.maxRetries else {
+                throw ImportParser.ImportError.badEncoding
+            }
+            return true
         }
         var dropHeader = false
 
@@ -118,14 +129,15 @@ public class InputHandler {
         try self.delegate?.append(records: dropHeader ? Array(rows.dropFirst()) : rows)
 
         self.data = Data()
+        self.retries = 0
 
         return true
     }
 
     /**
         Convenience for reading the FileHandle and handling parsing through to the end of file.
 
-        - Parameter length: Maximum byte length of data to read at each iteration.
+        - Parameter length: Maximum number of bytes of data to read at each iteration.
     */
     public func readToEndOfFile(length: Int = InputHandler.defaultByteLength) throws {
         while try self.read(length: length) {}
@@ -151,6 +163,7 @@ public class InputHandler {
         self.parser = ImportParser(dialect: dialect)
         self.isOpen = false
         self.data = Data()
+        self.retries = 0
         self.delegate?.reset()
         return true
     }

Tests/DialectalCSVTests/Fixtures/variableWidthEncodedStreamSplit.csv

@@ -0,0 +1,2 @@
+éab,abé,aéb,abcé
+123,456,789,321

Tests/DialectalCSVTests/Fixtures/western1252Encoded.csv

@@ -0,0 +1,2 @@
+"quote","author"
+"Always bear in mind that your own resol�tion to succeed is more important than any other.","Abraham Lincoln"

Tests/DialectalCSVTests/ImportTests.swift

@@ -21,6 +21,8 @@ class ImportTests: XCTestCase {
         ("testTrailingComma", testTrailingComma),
         ("testUnescapedQuotes", testUnescapedQuotes),
         ("testUnquotedHeaders", testUnquotedHeaders),
+        ("testBadEncoding", testBadEncoding),
+        ("testVariableWidthEncodedStreamSplit", testVariableWidthEncodedStreamSplit)
     ]
 
     func testEscapeCharacter() {
@@ -105,42 +107,26 @@ class ImportTests: XCTestCase {
         XCTAssertEqual(document.records.count, 1)
     }
 
-    func testLineTerminatorStreamSplit() {
+    func testLineTerminatorStreamSplit() throws {
         let inputURL = Utility.fixtureURL(named: "lineTerminatorStreamSplit.csv")
         var dialect = Dialect()
         dialect.header = false
-        let inputFileHandle = try! FileHandle(forReadingFrom: inputURL)
+        let inputFileHandle = try FileHandle(forReadingFrom: inputURL)
         let inputHandler = InputHandler(fileHandle: inputFileHandle, dialect: dialect)
 
-        class Handler: InputHandlerDelegate {
-            var records = [Record]()
-
-            public func open(header: Header? = nil) throws {}
-
-            public func append(records: [Record]) throws {
-                self.records.append(contentsOf: records)
-            }
-
-            public func close() throws {}
-
-            public func reset() {}
-        }
-        let handler = Handler()
+        let handler = SpyInputHandlerDelegate()
         inputHandler.delegate = handler
 
-        do {
-            try inputHandler.readToEndOfFile(length: 8)
-        } catch {
-            XCTFail()
-        }
-
+        try inputHandler.readToEndOfFile(length: 8)
         XCTAssertEqual(handler.records.count, 2)
 
-        XCTAssertEqual(handler.records[0][0], "abc")
-        XCTAssertEqual(handler.records[0][1], "xyz")
+        let first = handler.records[0]
+        XCTAssertEqual(first[0], "abc")
+        XCTAssertEqual(first[1], "xyz")
 
-        XCTAssertEqual(handler.records[1][0], "123")
-        XCTAssertEqual(handler.records[1][1], "456")
+        let second = handler.records[1]
+        XCTAssertEqual(second[0], "123")
+        XCTAssertEqual(second[1], "456")
     }
 
     func testMissingEndOfLine() {
@@ -304,4 +290,63 @@ class ImportTests: XCTestCase {
         XCTAssertEqual(document.header![1], HeaderFields.author.rawValue + " name")
     }
 
+    func testBadEncoding() throws {
+        let fileURL = Utility.fixtureURL(named: "western1252Encoded.csv")
+        let fileHandle = try FileHandle(forReadingFrom: fileURL)
+        do {
+            _ = try Document(fileHandle: fileHandle)
+        } catch ImportParser.ImportError.badEncoding {
+            return
+        } catch {
+            XCTFail()
+            return
+        }
+        XCTFail()
+    }
+
+    func testVariableWidthEncodedStreamSplit() throws {
+        let inputURL = Utility.fixtureURL(named: "variableWidthEncodedStreamSplit.csv")
+        var dialect = Dialect()
+        dialect.header = false
+
+        let inputFileHandle = try FileHandle(forReadingFrom: inputURL)
+        var inputHandler = InputHandler(fileHandle: inputFileHandle, dialect: dialect)
+        var handler = SpyInputHandlerDelegate()
+        inputHandler.delegate = handler
+
+        for numberOfBytes in 1...4 {
+            try inputHandler.readToEndOfFile(length: numberOfBytes)
+            XCTAssertEqual(handler.records.count, 2)
+
+            let first = try XCTUnwrap(handler.records[safe: 0])
+            XCTAssertEqual(first.count, 4)
+            XCTAssertEqual(first[0], "éab")
+            XCTAssertEqual(first[1], "abé")
+            XCTAssertEqual(first[2], "aéb")
+            XCTAssertEqual(first[3], "abcé")
+
+            let second = try XCTUnwrap(handler.records[safe: 1])
+            XCTAssertEqual(second.count, 4)
+            XCTAssertEqual(second[0], "123")
+            XCTAssertEqual(second[1], "456")
+            XCTAssertEqual(second[2], "789")
+            XCTAssertEqual(second[3], "321")
+        }
+
+        inputHandler = InputHandler(fileHandle: inputFileHandle, dialect: dialect, maxRetries: 0)
+        handler = SpyInputHandlerDelegate()
+        inputHandler.delegate = handler
+
+        for numberOfBytes in 1...4 {
+            do {
+                try inputHandler.readToEndOfFile(length: numberOfBytes)
+            } catch ImportParser.ImportError.badEncoding {
+                return
+            } catch {
+                XCTFail()
+                return
+            }
+        }
+    }
+
 }

Tests/DialectalCSVTests/Utility.swift

@@ -46,3 +46,30 @@ class Utility {
     }
 
 }
+
+extension Array {
+
+    subscript(safe index: Index) -> Element? {
+        guard index < endIndex, index >= startIndex else { return nil }
+        return self[index]
+    }
+
+}
+
+import DialectalCSV
+
+class SpyInputHandlerDelegate: InputHandlerDelegate {
+
+    private(set) var records = [Record]()
+
+    public func open(header: Header? = nil) throws {}
+
+    public func append(records: [Record]) throws {
+        self.records.append(contentsOf: records)
+    }
+
+    public func close() throws {}
+
+    public func reset() {}
+
+}