sftp: add single merge tests for both local <-> sftp and sftp <-> local

Had to fixup the symlink code in Sftp, most importantly add a PathUtils
relativeTo to fixup relative links.

7 files changed, 228 insertions, 6 deletions

diff --git a/libs/sftp/build.gradle.kts b/libs/sftp/build.gradle.kts
index 7b1afb9..e7efd26 100644
--- a/libs/sftp/build.gradle.kts
+++ b/libs/sftp/build.gradle.kts
@@ -65,6 +65,7 @@ android {
 dependencies {
     implementation(project(":libs:io"))
     implementation(project(":libs:utils"))
+    testImplementation(project(":libs:local"))
     testImplementation(project(":libs:test-utils"))
 }
 
diff --git a/libs/sftp/src/main/java/org/the_jk/cleversync/io/sftp/SftpConnection.kt b/libs/sftp/src/main/java/org/the_jk/cleversync/io/sftp/SftpConnection.kt
index ed5889a..6669e13 100644
--- a/libs/sftp/src/main/java/org/the_jk/cleversync/io/sftp/SftpConnection.kt
+++ b/libs/sftp/src/main/java/org/the_jk/cleversync/io/sftp/SftpConnection.kt
@@ -50,11 +50,10 @@ internal class SftpConnection(uri: Uri, credentials: SftpCredentials, hostsStora
         sftpSession?.unlink(PathUtils.join(baseDir, path)) ?: false
 
     fun readLink(path: String): String? {
-        val target = sftpSession?.readlink(PathUtils.join(baseDir, path))
-        if (target?.startsWith(baseDir) == true) {
-            return target.substring(baseDir.length + 1)
-        }
-        return target
+        val linkPath = PathUtils.join(baseDir, path)
+        val target = sftpSession?.readlink(linkPath) ?: return null
+        val parentPath = PathUtils.dirname(linkPath)
+        return PathUtils.relativeTo(parentPath, target)
     }
 
     fun symlink(target: String, rawTarget: Boolean, path: String): Boolean {
diff --git a/libs/sftp/src/test/java/org/the_jk/cleversync/sftp/SingleMergeLocalSftpTest.kt b/libs/sftp/src/test/java/org/the_jk/cleversync/sftp/SingleMergeLocalSftpTest.kt
new file mode 100644
index 0000000..0e61fa6
--- /dev/null
+++ b/libs/sftp/src/test/java/org/the_jk/cleversync/sftp/SingleMergeLocalSftpTest.kt
@@ -0,0 +1,76 @@
+package org.the_jk.cleversync.sftp
+
+import androidx.test.core.app.ApplicationProvider
+import com.google.common.truth.Truth.assertThat
+import org.junit.After
+import org.junit.AfterClass
+import org.junit.BeforeClass
+import org.junit.Rule
+import org.junit.rules.TemporaryFolder
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+import org.the_jk.cleversync.io.BaseSingleMergeTest
+import org.the_jk.cleversync.io.ModifiableTree
+import org.the_jk.cleversync.io.sftp.SftpCredentials
+import org.the_jk.cleversync.io.sftp.SftpHostsStorage
+import org.the_jk.cleversync.local.LocalTreeFactory
+
+@Config(manifest=Config.NONE)
+@RunWith(RobolectricTestRunner::class)
+class SingleMergeLocalSftpTest : BaseSingleMergeTest() {
+    @Rule
+    @JvmField
+    val temp = TemporaryFolder()
+
+    private lateinit var hostsStorage: SftpHostsStorage
+    private lateinit var sftp: ModifiableTree
+
+    @After
+    fun tearDownTest() {
+        sftp.close()
+
+        for (file in helper.shareDir.listFiles()!!) {
+            if (file.isDirectory) {
+                file.deleteRecursively()
+            } else {
+                file.delete()
+            }
+        }
+    }
+
+    override fun source(): ModifiableTree {
+        return LocalTreeFactory.modifiableTree(temp.root.toPath())
+    }
+
+    override fun sourceSupportsSymlinks() = true
+
+    override fun target(): ModifiableTree {
+        assertThat(helper.shareDir.listFiles()).isEmpty()
+
+        val credentials = SftpCredentials.SftpPasswordCredentials("user", "notverysecret")
+
+        hostsStorage = SftpHostsStorage(ApplicationProvider.getApplicationContext())
+
+        sftp = SftpTreeFactory.modifiableTree(helper.uri, credentials, hostsStorage).getOrThrow()
+        return sftp
+    }
+
+    override fun targetSupportsSymlinks() = true
+
+    companion object {
+        private val helper = SftpTestHelper()
+
+        @BeforeClass
+        @JvmStatic
+        fun setUpClass() {
+            helper.setUp()
+        }
+
+        @AfterClass
+        @JvmStatic
+        fun tearDownClass() {
+            helper.tearDown()
+        }
+    }
+}
diff --git a/libs/sftp/src/test/java/org/the_jk/cleversync/sftp/SingleMergeSftpLocalTest.kt b/libs/sftp/src/test/java/org/the_jk/cleversync/sftp/SingleMergeSftpLocalTest.kt
new file mode 100644
index 0000000..eeb473c
--- /dev/null
+++ b/libs/sftp/src/test/java/org/the_jk/cleversync/sftp/SingleMergeSftpLocalTest.kt
@@ -0,0 +1,77 @@
+package org.the_jk.cleversync.sftp
+
+import androidx.test.core.app.ApplicationProvider
+import com.google.common.truth.Truth.assertThat
+import org.junit.After
+import org.junit.AfterClass
+import org.junit.BeforeClass
+import org.junit.Rule
+import org.junit.rules.TemporaryFolder
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+import org.the_jk.cleversync.io.BaseSingleMergeTest
+import org.the_jk.cleversync.io.ModifiableTree
+import org.the_jk.cleversync.io.sftp.SftpCredentials
+import org.the_jk.cleversync.io.sftp.SftpHostsStorage
+import org.the_jk.cleversync.local.LocalTreeFactory
+import kotlin.time.Duration.Companion.milliseconds
+
+@Config(manifest=Config.NONE)
+@RunWith(RobolectricTestRunner::class)
+class SingleMergeSftpLocalTest : BaseSingleMergeTest() {
+    @Rule
+    @JvmField
+    val temp = TemporaryFolder()
+
+    private lateinit var hostsStorage: SftpHostsStorage
+    private lateinit var sftp: ModifiableTree
+
+    @After
+    fun tearDownTest() {
+        sftp.close()
+
+        for (file in helper.shareDir.listFiles()!!) {
+            if (file.isDirectory) {
+                file.deleteRecursively()
+            } else {
+                file.delete()
+            }
+        }
+    }
+
+    override fun source(): ModifiableTree {
+        assertThat(helper.shareDir.listFiles()).isEmpty()
+
+        val credentials = SftpCredentials.SftpPasswordCredentials("user", "notverysecret")
+
+        hostsStorage = SftpHostsStorage(ApplicationProvider.getApplicationContext())
+
+        sftp = SftpTreeFactory.modifiableTree(helper.uri, credentials, hostsStorage).getOrThrow()
+        return sftp
+    }
+
+    override fun sourceSupportsSymlinks() = true
+
+    override fun target(): ModifiableTree {
+        return LocalTreeFactory.modifiableTree(temp.root.toPath())
+    }
+
+    override fun targetSupportsSymlinks() = true
+
+    companion object {
+        private val helper = SftpTestHelper()
+
+        @BeforeClass
+        @JvmStatic
+        fun setUpClass() {
+            helper.setUp()
+        }
+
+        @AfterClass
+        @JvmStatic
+        fun tearDownClass() {
+            helper.tearDown()
+        }
+    }
+}
diff --git a/libs/test-utils/src/main/java/org/the_jk/cleversync/io/BaseSingleMergeTest.kt b/libs/test-utils/src/main/java/org/the_jk/cleversync/io/BaseSingleMergeTest.kt
index 156f978..2b53a06 100644
--- a/libs/test-utils/src/main/java/org/the_jk/cleversync/io/BaseSingleMergeTest.kt
+++ b/libs/test-utils/src/main/java/org/the_jk/cleversync/io/BaseSingleMergeTest.kt
@@ -156,7 +156,6 @@ abstract class BaseSingleMergeTest {
 
         src.createLink("link1", srcFoo)
         src.createLink("link2", "does-not-exist")
-        src.createLink("link3", "../../escaped")
 
         val actions = SingleMerge.calculate(tgt, src)
         assertThat(actions).containsExactly(
@@ -183,6 +182,38 @@ abstract class BaseSingleMergeTest {
     }
 
     @Test
+    fun symlinkOutsideRoot() {
+        Assume.assumeTrue(sourceSupportsSymlinks())
+
+        val link = src.createLink("bad_link", "../../escaped")
+
+        // Some sources doesn't allow to create symlinks that point outside filesystem
+        when (val target = link.resolve()) {
+            is Link.NoTarget -> Assume.assumeTrue(target.path == "../../escaped")
+            is Link.FileTarget,
+            is Link.DirectoryTarget,
+                -> Assert.fail("Should not target anything")
+        }
+
+
+        val actions = SingleMerge.calculate(tgt, src)
+        assertThat(actions).isEmpty()
+    }
+
+    @Test
+    fun symlinkOutsideDir() {
+        Assume.assumeTrue(sourceSupportsSymlinks())
+
+        val dir = src.createDirectory("foo")
+        val escaped = src.createDirectory("escaped")
+        dir.createLink("bad_link", escaped)
+        src.removeDirectory("escaped")
+
+        val actions = SingleMerge.calculate(tgt, dir)
+        assertThat(actions).isEmpty()
+    }
+
+    @Test
     fun resolveSymlink() {
         Assume.assumeTrue(sourceSupportsSymlinks())
 
diff --git a/libs/utils/src/main/java/org/the_jk/cleversync/PathUtils.kt b/libs/utils/src/main/java/org/the_jk/cleversync/PathUtils.kt
index 4202b24..77037cd 100644
--- a/libs/utils/src/main/java/org/the_jk/cleversync/PathUtils.kt
+++ b/libs/utils/src/main/java/org/the_jk/cleversync/PathUtils.kt
@@ -1,5 +1,7 @@
 package org.the_jk.cleversync
 
+import kotlin.math.min
+
 object PathUtils {
     fun dirname(path: String): String {
         if (path.isEmpty()) return "."
@@ -46,4 +48,29 @@ object PathUtils {
         }
         return parts.joinToString("/", prefix = if (prefixed) "/" else "")
     }
+
+    // Return path relative to base. Returns path as-is if either path is not
+    // absolute.
+    fun relativeTo(base: String, path: String): String {
+        if (!base.startsWith('/') || !path.startsWith('/')) return path
+        val baseParts = base.split('/').filter { part ->
+            part.isNotEmpty() && part != "."
+        }
+        val pathParts = path.split('/').filter { part ->
+            part.isNotEmpty() && part != "."
+        }
+        var i = 0
+        while (i < min(baseParts.size, pathParts.size)) {
+            if (baseParts[i] != pathParts[i]) break
+            i++
+        }
+        if (i == baseParts.size) {
+            if (i == pathParts.size) return "."
+            return pathParts.subList(i, pathParts.size).joinToString("/")
+        }
+        return buildList {
+            repeat(baseParts.size - i) { add("..") }
+            addAll(pathParts.subList(i, pathParts.size))
+        }.joinToString("/")
+    }
 }
diff --git a/libs/utils/src/test/java/org/the_jk/cleversync/PathUtilsTest.kt b/libs/utils/src/test/java/org/the_jk/cleversync/PathUtilsTest.kt
index eb1d63f..1d411ea 100644
--- a/libs/utils/src/test/java/org/the_jk/cleversync/PathUtilsTest.kt
+++ b/libs/utils/src/test/java/org/the_jk/cleversync/PathUtilsTest.kt
@@ -55,4 +55,15 @@ class PathUtilsTest {
         assertThat(PathUtils.resolve("foo/../../../bar/")).isEqualTo("bar")
         assertThat(PathUtils.resolve("/foo/../../../bar/")).isEqualTo("/bar")
     }
+
+    @Test
+    fun relativeTo() {
+        assertThat(PathUtils.relativeTo("", "")).isEmpty()
+        assertThat(PathUtils.relativeTo("/", "/")).isEqualTo(".")
+        assertThat(PathUtils.relativeTo("/", "/foo")).isEqualTo("foo")
+        assertThat(PathUtils.relativeTo("/", "/foo/bar/")).isEqualTo("foo/bar")
+        assertThat(PathUtils.relativeTo("/foo", "/foo/bar/")).isEqualTo("bar")
+        assertThat(PathUtils.relativeTo("/foo", "/bar")).isEqualTo("../bar")
+        assertThat(PathUtils.relativeTo("/foo/bar", "/fum/bar")).isEqualTo("../../fum/bar")
+    }
 }