Merge pull request #83 from weichsel/bugfix/implicitlyUnwrappedFileEr…

…rorHandling Add guards to POSIX fopen calls
weichsel · Dec 15, 2018 · 2a527b9 · 2a527b9
2 parents e933784 + f5ecf3a
commit 2a527b9
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 10 deletions.
diff --git a/Sources/ZIPFoundation/Archive+Reading.swift b/Sources/ZIPFoundation/Archive+Reading.swift
@@ -30,8 +30,10 @@ extension Archive {
                 throw CocoaError.error(.fileWriteFileExists, userInfo: [NSFilePathErrorKey: url.path], url: nil)
             }
             try fileManager.createParentDirectoryStructure(for: url)
-            let destinationFileSystemRepresentation = fileManager.fileSystemRepresentation(withPath: url.path)
-            let destinationFile: UnsafeMutablePointer<FILE> = fopen(destinationFileSystemRepresentation, "wb+")
+            let destinationRepresentation = fileManager.fileSystemRepresentation(withPath: url.path)
+            guard let destinationFile: UnsafeMutablePointer<FILE> = fopen(destinationRepresentation, "wb+") else {
+                throw CocoaError.error(.fileNoSuchFile)
+            }
             defer { fclose(destinationFile) }
             let consumer = { _ = try Data.write(chunk: $0, to: destinationFile) }
             checksum = try self.extract(entry, bufferSize: bufferSize, progress: progress, consumer: consumer)

diff --git a/Sources/ZIPFoundation/Archive+Writing.swift b/Sources/ZIPFoundation/Archive+Writing.swift
@@ -44,7 +44,9 @@ extension Archive {
         switch type {
         case .file:
             let entryFileSystemRepresentation = fileManager.fileSystemRepresentation(withPath: entryURL.path)
-            let entryFile: UnsafeMutablePointer<FILE> = fopen(entryFileSystemRepresentation, "rb")
+            guard let entryFile: UnsafeMutablePointer<FILE> = fopen(entryFileSystemRepresentation, "rb") else {
+                throw CocoaError.error(.fileNoSuchFile)
+            }
             defer { fclose(entryFile) }
             provider = { _, _ in return try Data.readChunk(of: Int(bufferSize), from: entryFile) }
             try self.addEntry(with: path, type: type, uncompressedSize: uncompressedSize,
@@ -60,7 +62,6 @@ extension Archive {
                               progress: progress, provider: provider)
         case .symlink:
             provider = { _, _ -> Data in
-                let fileManager = FileManager()
                 let linkDestination = try fileManager.destinationOfSymbolicLink(atPath: entryURL.path)
                 let linkFileSystemRepresentation = fileManager.fileSystemRepresentation(withPath: linkDestination)
                 let linkLength = Int(strlen(linkFileSystemRepresentation))

diff --git a/Sources/ZIPFoundation/Archive.swift b/Sources/ZIPFoundation/Archive.swift
@@ -141,10 +141,11 @@ public final class Archive: Sequence {
             guard fileManager.fileExists(atPath: url.path) else { return nil }
             guard fileManager.isReadableFile(atPath: url.path) else { return nil }
             let fileSystemRepresentation = fileManager.fileSystemRepresentation(withPath: url.path)
-            self.archiveFile = fopen(fileSystemRepresentation, "rb")
-            guard let endOfCentralDirectoryRecord = Archive.scanForEndOfCentralDirectoryRecord(in: archiveFile) else {
+            guard let archiveFile = fopen(fileSystemRepresentation, "rb"),
+                let endOfCentralDirectoryRecord = Archive.scanForEndOfCentralDirectoryRecord(in: archiveFile) else {
                 return nil
             }
+            self.archiveFile = archiveFile
             self.endOfCentralDirectoryRecord = endOfCentralDirectoryRecord
         case .create:
             guard !fileManager.fileExists(atPath: url.path) else { return nil }
@@ -161,10 +162,11 @@ public final class Archive: Sequence {
         case .update:
             guard fileManager.isWritableFile(atPath: url.path) else { return nil }
             let fileSystemRepresentation = fileManager.fileSystemRepresentation(withPath: url.path)
-            self.archiveFile = fopen(fileSystemRepresentation, "rb+")
-            guard let endOfCentralDirectoryRecord = Archive.scanForEndOfCentralDirectoryRecord(in: archiveFile) else {
+            guard let archiveFile = fopen(fileSystemRepresentation, "rb+"),
+                let endOfCentralDirectoryRecord = Archive.scanForEndOfCentralDirectoryRecord(in: archiveFile) else {
                 return nil
             }
+            self.archiveFile = archiveFile
             self.endOfCentralDirectoryRecord = endOfCentralDirectoryRecord
             fseek(self.archiveFile, 0, SEEK_SET)
         }
@@ -188,7 +190,7 @@ public final class Archive: Sequence {
             let offset = Int(centralDirStruct.relativeOffsetOfLocalHeader)
             guard let localFileHeader: LocalFileHeader = Data.readStruct(from: self.archiveFile,
                                                                          at: offset) else { return nil }
-            var dataDescriptor: DataDescriptor? = nil
+            var dataDescriptor: DataDescriptor?
             if centralDirStruct.usesDataDescriptor {
                 let additionalSize = Int(localFileHeader.fileNameLength + localFileHeader.extraFieldLength)
                 let isCompressed = centralDirStruct.compressionMethod != CompressionMethod.none.rawValue

diff --git a/Sources/ZIPFoundation/Data+Compression.swift b/Sources/ZIPFoundation/Data+Compression.swift
@@ -176,7 +176,6 @@ extension Data {
                     try consumer(outputData)
                     if operation == COMPRESSION_STREAM_DECODE { checksum = outputData.crc32(checksum: checksum) }
                 }
-            case COMPRESSION_STATUS_ERROR: fallthrough
             default: throw CompressionError.corruptedData
             }
         } while (status == COMPRESSION_STATUS_OK)

diff --git a/Tests/ZIPFoundationTests/ZIPFoundationReadingTests.swift b/Tests/ZIPFoundationTests/ZIPFoundationReadingTests.swift
@@ -118,6 +118,17 @@ extension ZIPFoundationTests {
             XCTFail("Failed to obtain test asset from archive.")
             return
         }
+        do {
+            let longFileName = String(repeating: ProcessInfo.processInfo.globallyUniqueString, count: 100)
+            var overlongURL = URL(fileURLWithPath: NSTemporaryDirectory())
+            overlongURL.appendPathComponent(longFileName)
+            _ = try archive.extract(fileEntry, to: overlongURL)
+        } catch let error as CocoaError {
+            XCTAssert(error.code == CocoaError.fileNoSuchFile)
+        } catch {
+            XCTFail("Unexpected error while trying to extract entry to invalid URL.")
+            return
+        }
         XCTAssertNotNil(linkEntry)
         do {
             _ = try archive.extract(linkEntry, to: archive.url)