Fixes for overwrite mode on SFTP during copy operation

- Removed flags from BlinkFiles::create. It was used as a way to pass it down to
the open but it made things more confusing.
- Create now uses by default the expected flags everywhere.
- Additional tests on SFTP.

Author: Carlos Cabanero

Blink/Commands/ssh/CopyFiles.swift

@@ -252,7 +252,7 @@ public class BlinkCopy: NSObject {
               let newFileName = (self.command.destination.filePath as NSString).lastPathComponent
               let parentPath = (self.command.destination.filePath as NSString).deletingLastPathComponent
               return d.cloneWalkTo(parentPath)
-                .flatMap { $0.create(name: newFileName, flags: O_WRONLY, mode: S_IRWXU) }
+                .flatMap { $0.create(name: newFileName, mode: S_IRWXU) }
                 .flatMap { $0.close() }
                 .flatMap { _ in d.cloneWalkTo(self.command.destination.filePath) }
                 .eraseToAnyPublisher()

BlinkCode/CodeFileSystem.swift

@@ -164,7 +164,7 @@ class CodeFileSystem {
             if !(options.create ?? false) {
               return .fail(error: CodeFileSystemError.fileNotFound(uri: self.uri))
             }
-            return parentT.create(name: fileName, flags: O_WRONLY, mode: 0o644)
+            return parentT.create(name: fileName, mode: 0o644)
           }
         // 2. Write the content to the file
           .flatMap { file -> AnyPublisher<Int, Error> in

BlinkFiles/BlinkFiles.swift

@@ -62,7 +62,7 @@ public protocol Translator: CopierFrom {
 
   // Creates the file name at the current walked path with the given permissions.
   // Default mode  = S_IRWXU
-  func create(name: String, flags: Int32, mode: mode_t) -> AnyPublisher<File, Error>
+  func create(name: String, mode: mode_t) -> AnyPublisher<File, Error>
   // Creates the directory name at the current walked path with the given permissions.
   // Default mode =  S_IRWUX | S_IRWXG | S_IRWXO
   func mkdir(name: String, mode: mode_t) -> AnyPublisher<Translator, Error>

BlinkFiles/CopyFiles.swift

@@ -201,10 +201,10 @@ extension Translator {
 
     let fullFile: String
     let file: AnyPublisher<File, Error>
-    // If we are in a directory, we create the file, otherwise we open truncated.
+    // If we are a directory, we create the file. If we are a file, we open truncated.
     if self.isDirectory {
       fullFile = (self.current as NSString).appendingPathComponent(name)
-      file = self.create(name: name, flags: O_WRONLY, mode: S_IRWXU)
+      file = self.create(name: name, mode: S_IRWXU)
     } else {
       fullFile = self.current
       file = self.open(flags: O_WRONLY | O_TRUNC)

BlinkFiles/LocalFiles.swift

@@ -134,8 +134,7 @@ public class Local : Translator {
       }.eraseToAnyPublisher()
   }
 
-  //    // TODO Change permissions to more generic open options
-  public func create(name: String, flags: Int32, mode: mode_t = S_IRWXU) -> AnyPublisher<File, Error> {
+  public func create(name: String, mode: mode_t = S_IRWXU) -> AnyPublisher<File, Error> {
     if fileType != .typeDirectory {
       return fail(msg: "Not a directory.")
     }
@@ -148,7 +147,7 @@ public class Local : Translator {
         throw LocalFileError(msg: "Could not create file.")
       }
 
-      return try LocalFile(at: absPath, flags: flags)
+      return try LocalFile(at: absPath, flags: O_WRONLY|O_CREAT|O_TRUNC)
     }.eraseToAnyPublisher()
   }
 

BlinkFilesTests/LocalFilesTests.swift

@@ -176,7 +176,7 @@ class LocalFilesTests: XCTestCase {
     f.walkTo("/Users/carlos/Xcode_12.0.1.xip")
       .flatMap { $0.open(flags: O_RDONLY) }
       .flatMap { srcFile -> AnyPublisher<Int, Error> in
-        return dst.create(name: "Docker-copy.dmg", flags: O_WRONLY, mode: 0o644)
+        return dst.create(name: "Docker-copy.dmg", mode: 0o644)
           .flatMap { dstFile in
             return (srcFile as! WriterTo).writeTo(dstFile)
           }.eraseToAnyPublisher()

SSH/SCP.swift

@@ -383,7 +383,7 @@ extension SCPClient {
 
   // Flow when receiving a file to copy. Create on Translator and then Write to it.
   func copyFileTo(translator: Translator, usingName name: String, length: UInt64, mode: mode_t) -> CopyProgressInfoPublisher {
-    return translator.create(name: name, flags: O_RDWR, mode: mode).flatMap { file -> CopyProgressInfoPublisher in
+    return translator.create(name: name, mode: mode).flatMap { file -> CopyProgressInfoPublisher in
       var totalWritten: UInt64 = 0
 
       return self.writeTo(file, length: length).map { written in

SSH/SFTP.swift

@@ -247,7 +247,7 @@ public class SFTPTranslator: BlinkFiles.Translator {
     }.eraseToAnyPublisher()
   }
 
-  public func create(name: String, flags: Int32, mode: mode_t = S_IRWXU) -> AnyPublisher<BlinkFiles.File, Error> {
+  public func create(name: String, mode: mode_t = S_IRWXU) -> AnyPublisher<BlinkFiles.File, Error> {
     if fileType != .typeDirectory {
       return .fail(error: FileError(title: "Not a directory.", in: session))
     }
@@ -257,7 +257,7 @@ public class SFTPTranslator: BlinkFiles.Translator {
       defer { ssh_channel_set_blocking(self.channel, 0) }
 
       let filePath = (self.path as NSString).appendingPathComponent(name)
-      guard let file = sftp_open(sftp, filePath, flags | O_CREAT, mode) else {
+      guard let file = sftp_open(sftp, filePath, O_WRONLY|O_CREAT|O_TRUNC, mode) else {
         throw FileError(in: self.session)
       }
 

SSHTests/SFTPTests.swift

@@ -133,19 +133,23 @@ class SFTPTests: XCTestCase {
   }
 
   func testWrite() throws {
-    let expectation = self.expectation(description: "Buffer Written")
+    let writeExpectation = self.expectation(description: "File Written")
 
     //var connection: SSHClient?
-    //var sftp: SFTPClient?
+    var root: SFTPTranslator? = nil
     var totalWritten = 0
 
     let gen = RandomInputGenerator(fast: true)
 
-    let cancellable = SSHClient.dialWithTestConfig()
+    let cancelWrite = SSHClient.dialWithTestConfig()
       .flatMap() { $0.requestSFTP() }
-      .tryMap()  { try SFTPTranslator(on: $0) }
+      .tryMap()  { t in
+        let r = try SFTPTranslator(on: t)
+        root = r
+        return r
+      }
       .flatMap() { $0.walkTo("/tmp") }
-      .flatMap() { $0.create(name: "newfile", flags: O_WRONLY, mode: S_IRWXU) }
+      .flatMap() { $0.create(name: "newfile", mode: S_IRWXU) }
       .flatMap() { file in
         return gen.read(max: 5 * 1024 * 1024)
           .flatMap() { data in
@@ -154,7 +158,7 @@ class SFTPTests: XCTestCase {
       }.sink(receiveCompletion: { completion in
         switch completion {
         case .finished:
-          expectation.fulfill()
+          writeExpectation.fulfill()
         case .failure(let error):
           // Problem here is we can have both SFTP and SSHError
           if let err = error as? SSH.FileError {
@@ -169,6 +173,51 @@ class SFTPTests: XCTestCase {
 
     waitForExpectations(timeout: 15, handler: nil)
     XCTAssert(totalWritten == 5 * 1024 * 1024, "Did not write all data")
+    
+    totalWritten = 0
+    let overwriteExpectation = self.expectation(description: "File Overwritten")
+    
+    guard let root = root else {
+      XCTFail("No root translator.")
+      return
+    }
+    
+    let cancelOverwrite = root.walkTo("/tmp")
+      .flatMap() { $0.create(name: "newfile", mode: S_IRWXU) }
+      .flatMap() { file in
+        return gen.read(max: 4 * 1024 * 1024)
+          .flatMap() { data in
+            return file.write(data, max: data.count)
+          }
+      }.sink(receiveCompletion: { completion in
+        switch completion {
+        case .finished:
+          overwriteExpectation.fulfill()
+        case .failure(let error):
+          // Problem here is we can have both SFTP and SSHError
+          if let err = error as? SSH.FileError {
+            XCTFail(err.description)
+          } else {
+            XCTFail("Crash")
+          }
+        }
+      }, receiveValue: { written in
+        totalWritten += written
+      })
+    
+    waitForExpectations(timeout: 15, handler: nil)
+    XCTAssert(totalWritten == 4 * 1024 * 1024, "Did not write all data")
+    
+    let statExpectation = self.expectation(description: "File Stat")
+    let cancelStat = root.walkTo("/tmp/newfile")
+      .flatMap { (t: Translator) -> AnyPublisher<FileAttributes, Error> in t.stat() }
+      .assertNoFailure()
+      .sink { (stats: FileAttributes) in
+        XCTAssertTrue(stats[.size] as! Int == 4 * 1024 * 1024)
+        statExpectation.fulfill()
+      }
+    
+    waitForExpectations(timeout: 15, handler: nil)
   }
 
   func testWriteToWriter() throws {
@@ -190,7 +239,7 @@ class SFTPTests: XCTestCase {
       }.flatMap() { f -> AnyPublisher<Int, Error> in
         let file = f as! SFTPFile
         return translator!.walkTo("/tmp/")
-          .flatMap { $0.create(name: "linux.tar.xz", flags: O_WRONLY, mode: S_IRWXU) }
+          .flatMap { $0.create(name: "linux.tar.xz", mode: S_IRWXU) }
           .flatMap() { file.writeTo($0) }.eraseToAnyPublisher()
       }.sink(receiveCompletion: { completion in
         switch completion {