Merge pull request #681 from grote/webdav-trailing-slash

Improve WebDAV compatibility

app/src/main/java/com/stevesoltys/seedvault/plugins/StoragePluginManager.kt

@@ -139,7 +139,7 @@ class StoragePluginManager(
     suspend fun getFreeSpace(): Long? {
         return try {
             appPlugin.getFreeSpace()
-        } catch (e: Exception) {
+        } catch (e: Throwable) { // NoClassDefFound isn't an [Exception], can get thrown by dav4jvm
             Log.e("StoragePluginManager", "Error getting free space: ", e)
             null
         }

app/src/main/java/com/stevesoltys/seedvault/plugins/webdav/WebDavStorage.kt

@@ -8,7 +8,15 @@ package com.stevesoltys.seedvault.plugins.webdav
 import android.util.Log
 import at.bitfire.dav4jvm.BasicDigestAuthHandler
 import at.bitfire.dav4jvm.DavCollection
+import at.bitfire.dav4jvm.MultiResponseCallback
+import at.bitfire.dav4jvm.Property
+import at.bitfire.dav4jvm.PropertyFactory
+import at.bitfire.dav4jvm.PropertyRegistry
 import at.bitfire.dav4jvm.Response
+import at.bitfire.dav4jvm.Response.HrefRelation.SELF
+import at.bitfire.dav4jvm.XmlUtils.NS_WEBDAV
+import at.bitfire.dav4jvm.exception.HttpException
+import at.bitfire.dav4jvm.property.DisplayName
 import at.bitfire.dav4jvm.property.ResourceType
 import kotlinx.coroutines.DelicateCoroutinesApi
 import kotlinx.coroutines.Dispatchers
@@ -23,6 +31,7 @@ import okhttp3.OkHttpClient
 import okhttp3.RequestBody
 import okhttp3.internal.closeQuietly
 import okio.BufferedSink
+import org.xmlpull.v1.XmlPullParser
 import java.io.IOException
 import java.io.InputStream
 import java.io.OutputStream
@@ -65,6 +74,10 @@ internal abstract class WebDavStorage(
     protected val baseUrl = webDavConfig.url
     protected val url = "${webDavConfig.url}/$root"
 
+    init {
+        PropertyRegistry.register(GetLastModified.Factory)
+    }
+
     @Throws(IOException::class)
     protected suspend fun getOutputStream(location: HttpUrl): OutputStream {
         val davCollection = DavCollection(okHttpClient, location)
@@ -124,6 +137,59 @@ internal abstract class WebDavStorage(
         return pipedInputStream
     }
 
+    /**
+     * Tries to do [DavCollection.propfind] with a depth of `2` which is not in RFC4918.
+     * Since `infinity` isn't supported by nginx either,
+     * we fallback to iterating over all folders found with depth `1`
+     * and do another PROPFIND on those, passing the given [callback].
+     */
+    protected fun DavCollection.propfindDepthTwo(callback: MultiResponseCallback) {
+        try {
+            propfind(
+                depth = 2, // this isn't defined in RFC4918
+                reqProp = arrayOf(DisplayName.NAME, ResourceType.NAME),
+                callback = callback,
+            )
+        } catch (e: HttpException) {
+            if (e.isUnsupportedPropfind()) {
+                Log.i(TAG, "Got ${e.response}, trying two depth=1 PROPFINDs...")
+                propfindFakeTwo(callback)
+            } else {
+                throw e
+            }
+        }
+    }
+
+    private fun DavCollection.propfindFakeTwo(callback: MultiResponseCallback) {
+        propfind(
+            depth = 1,
+            reqProp = arrayOf(DisplayName.NAME, ResourceType.NAME),
+        ) { response, relation ->
+            debugLog { "propFindFakeTwo() = $response" }
+            // This callback will be called for everything in the folder
+            callback.onResponse(response, relation)
+            if (relation != SELF && response.isFolder()) {
+                DavCollection(okHttpClient, response.href).propfind(
+                    depth = 1,
+                    reqProp = arrayOf(DisplayName.NAME, ResourceType.NAME),
+                    callback = callback,
+                )
+            }
+        }
+    }
+
+    protected fun HttpException.isUnsupportedPropfind(): Boolean {
+        // nginx returns 400 for depth=2
+        if (code == 400) {
+            return true
+        }
+        // lighttpd returns 403 with <DAV:propfind-finite-depth/> error as if we used infinity
+        if (code == 403 && responseBody?.contains("propfind-finite-depth") == true) {
+            return true
+        }
+        return false
+    }
+
     protected suspend fun DavCollection.createFolder(xmlBody: String? = null): okhttp3.Response {
         return try {
             suspendCoroutine { cont ->
@@ -180,3 +246,19 @@ internal abstract class WebDavStorage(
     }
 
 }
+
+/**
+ * A fake version of [at.bitfire.dav4jvm.property.GetLastModified] which we register
+ * so we don't need to depend on `org.apache.commons.lang3` which is used for date parsing.
+ */
+class GetLastModified : Property {
+    companion object {
+        @JvmField
+        val NAME = Property.Name(NS_WEBDAV, "getlastmodified")
+    }
+
+    object Factory : PropertyFactory {
+        override fun getName() = NAME
+        override fun create(parser: XmlPullParser): GetLastModified? = null
+    }
+}

app/src/main/java/com/stevesoltys/seedvault/plugins/webdav/WebDavStoragePlugin.kt

@@ -9,6 +9,7 @@ import android.content.Context
 import android.util.Log
 import at.bitfire.dav4jvm.DavCollection
 import at.bitfire.dav4jvm.Response.HrefRelation.SELF
+import at.bitfire.dav4jvm.exception.HttpException
 import at.bitfire.dav4jvm.exception.NotFoundException
 import at.bitfire.dav4jvm.property.DisplayName
 import at.bitfire.dav4jvm.property.QuotaAvailableBytes
@@ -34,7 +35,7 @@ internal class WebDavStoragePlugin(
 ) : WebDavStorage(webDavConfig, root), StoragePlugin<WebDavConfig> {
 
     override suspend fun test(): Boolean {
-        val location = baseUrl.toHttpUrl()
+        val location = (if (baseUrl.endsWith('/')) baseUrl else "$baseUrl/").toHttpUrl()
         val davCollection = DavCollection(okHttpClient, location)
 
         val webDavSupported = suspendCoroutine { cont ->
@@ -50,7 +51,7 @@ internal class WebDavStoragePlugin(
     }
 
     override suspend fun getFreeSpace(): Long? {
-        val location = url.toHttpUrl()
+        val location = "$url/".toHttpUrl()
         val davCollection = DavCollection(okHttpClient, location)
 
         val availableBytes = suspendCoroutine { cont ->
@@ -70,7 +71,7 @@ internal class WebDavStoragePlugin(
 
     @Throws(IOException::class)
     override suspend fun startNewRestoreSet(token: Long) {
-        val location = "$url/$token".toHttpUrl()
+        val location = "$url/$token/".toHttpUrl()
         val davCollection = DavCollection(okHttpClient, location)
 
         val response = davCollection.createFolder()
@@ -81,7 +82,7 @@ internal class WebDavStoragePlugin(
     override suspend fun initializeDevice() {
         // TODO does it make sense to delete anything
         //  when [startNewRestoreSet] is always called first? Maybe unify both calls?
-        val location = url.toHttpUrl()
+        val location = "$url/".toHttpUrl()
         val davCollection = DavCollection(okHttpClient, location)
 
         try {
@@ -162,32 +163,37 @@ internal class WebDavStoragePlugin(
     override suspend fun getAvailableBackups(): Sequence<EncryptedMetadata>? {
         return try {
             doGetAvailableBackups()
-        } catch (e: Exception) {
+        } catch (e: Throwable) { // NoClassDefFound isn't an [Exception], can get thrown by dav4jvm
             Log.e(TAG, "Error getting available backups: ", e)
             null
         }
     }
 
     private suspend fun doGetAvailableBackups(): Sequence<EncryptedMetadata> {
-        val location = url.toHttpUrl()
+        val location = "$url/".toHttpUrl()
         val davCollection = DavCollection(okHttpClient, location)
 
         // get all restore set tokens in root folder
         val tokens = ArrayList<Long>()
-        davCollection.propfind(
+        try {
-            depth = 2,
+            davCollection.propfind(
-            reqProp = arrayOf(DisplayName.NAME, ResourceType.NAME),
+                depth = 2,
-        ) { response, relation ->
+                reqProp = arrayOf(DisplayName.NAME, ResourceType.NAME),
-            debugLog { "getAvailableBackups() = $response" }
+            ) { response, relation ->
-            // This callback will be called for every file in the folder
+                debugLog { "getAvailableBackups() = $response" }
-            if (relation != SELF && !response.isFolder() && response.href.pathSize >= 2 &&
+                // This callback will be called for every file in the folder
-                response.hrefName() == FILE_BACKUP_METADATA
+                if (relation != SELF && !response.isFolder() && response.href.pathSize >= 2 &&
-            ) {
+                    response.hrefName() == FILE_BACKUP_METADATA
-                val tokenName = response.href.pathSegments[response.href.pathSegments.size - 2]
+                ) {
-                getTokenOrNull(tokenName)?.let { token ->
+                    val tokenName = response.href.pathSegments[response.href.pathSegments.size - 2]
-                    tokens.add(token)
+                    getTokenOrNull(tokenName)?.let { token ->
+                        tokens.add(token)
+                    }
                 }
             }
+        } catch (e: HttpException) {
+            if (e.isUnsupportedPropfind()) getBackupTokenWithDepthOne(davCollection, tokens)
+            else throw e
         }
         val tokenIterator = tokens.iterator()
         return generateSequence {
@@ -199,6 +205,34 @@ internal class WebDavStoragePlugin(
         }
     }
 
+    private fun getBackupTokenWithDepthOne(davCollection: DavCollection, tokens: ArrayList<Long>) {
+        davCollection.propfind(
+            depth = 1,
+            reqProp = arrayOf(DisplayName.NAME, ResourceType.NAME),
+        ) { response, relation ->
+            debugLog { "getBackupTokenWithDepthOne() = $response" }
+
+            // we are only interested in sub-folders, skip rest
+            if (relation == SELF || !response.isFolder()) return@propfind
+
+            val token = getTokenOrNull(response.hrefName()) ?: return@propfind
+            val tokenUrl = response.href.newBuilder()
+                .addPathSegment(FILE_BACKUP_METADATA)
+                .build()
+            // check if .backup.metadata file exists using HEAD request,
+            // because some servers (e.g. nginx don't list hidden files with PROPFIND)
+            try {
+                DavCollection(okHttpClient, tokenUrl).head {
+                    debugLog { "getBackupTokenWithDepthOne() = $response" }
+                    tokens.add(token)
+                }
+            } catch (e: Exception) {
+                // just log exception and continue, we want to find all files that are there
+                Log.e(TAG, "Error retrieving $tokenUrl: ", e)
+            }
+        }
+    }
+
     private fun getTokenOrNull(name: String): Long? {
         val looksLikeToken = name.isNotEmpty() && tokenRegex.matches(name)
         if (looksLikeToken) {

app/src/main/java/com/stevesoltys/seedvault/storage/WebDavStoragePlugin.kt

@@ -49,7 +49,7 @@ internal class WebDavStoragePlugin(
 
     @Throws(IOException::class)
     override suspend fun init() {
-        val location = url.toHttpUrl()
+        val location = "$url/".toHttpUrl()
         val davCollection = DavCollection(okHttpClient, location)
 
         try {
@@ -67,7 +67,7 @@ internal class WebDavStoragePlugin(
 
     @Throws(IOException::class)
     override suspend fun getAvailableChunkIds(): List<String> {
-        val location = "$url/$folder".toHttpUrl()
+        val location = "$url/$folder/".toHttpUrl()
         val davCollection = DavCollection(okHttpClient, location)
         debugLog { "getAvailableChunkIds($location)" }
 
@@ -77,10 +77,7 @@ internal class WebDavStoragePlugin(
         val chunkIds = ArrayList<String>()
         try {
             val duration = measureDuration {
-                davCollection.propfind(
+                davCollection.propfindDepthTwo { response, relation ->
-                    depth = 2,
-                    reqProp = arrayOf(DisplayName.NAME, ResourceType.NAME),
-                ) { response, relation ->
                     debugLog { "getAvailableChunkIds() = $response" }
                     // This callback will be called for every file in the folder
                     if (relation != SELF && response.isFolder()) {
@@ -117,7 +114,7 @@ internal class WebDavStoragePlugin(
     ) {
         val s = missingChunkFolders.size
         for ((i, chunkFolderName) in missingChunkFolders.withIndex()) {
-            val location = "$url/$folder/$chunkFolderName".toHttpUrl()
+            val location = "$url/$folder/$chunkFolderName/".toHttpUrl()
             val davCollection = DavCollection(okHttpClient, location)
             val response = davCollection.createFolder()
             debugLog { "Created missing folder $chunkFolderName (${i + 1}/$s) $response" }
@@ -156,19 +153,16 @@ internal class WebDavStoragePlugin(
 
     @Throws(IOException::class)
     override suspend fun getBackupSnapshotsForRestore(): List<StoredSnapshot> {
-        val location = url.toHttpUrl()
+        val location = "$url/".toHttpUrl()
         val davCollection = DavCollection(okHttpClient, location)
         debugLog { "getBackupSnapshotsForRestore($location)" }
 
         val snapshots = ArrayList<StoredSnapshot>()
         try {
-            davCollection.propfind(
+            davCollection.propfindDepthTwo { response, relation ->
-                depth = 2,
-                reqProp = arrayOf(DisplayName.NAME, ResourceType.NAME),
-            ) { response, relation ->
                 debugLog { "getBackupSnapshotsForRestore() = $response" }
                 // This callback will be called for every file in the folder
-                if (relation != SELF && !response.isFolder()) {
+                if (relation != SELF && !response.isFolder() && response.href.pathSize >= 2) {
                     val name = response.hrefName()
                     val match = snapshotRegex.matchEntire(name)
                     if (match != null) {
@@ -220,7 +214,7 @@ internal class WebDavStoragePlugin(
 
     @Throws(IOException::class)
     override suspend fun getCurrentBackupSnapshots(): List<StoredSnapshot> {
-        val location = "$url/$folder".toHttpUrl()
+        val location = "$url/$folder/".toHttpUrl()
         val davCollection = DavCollection(okHttpClient, location)
         debugLog { "getCurrentBackupSnapshots($location)" }
 

app/src/test/java/com/stevesoltys/seedvault/plugins/webdav/WebDavStoragePluginTest.kt

@@ -71,6 +71,10 @@ internal class WebDavStoragePluginTest : TransportTest() {
         plugin.getOutputStream(token, FILE_BACKUP_METADATA).use {
             it.write(metadata)
         }
+
+        // now we have data
+        assertTrue(plugin.hasData(token, FILE_BACKUP_METADATA))
+
         try {
             // now we have one backup matching our token
             val backups = plugin.getAvailableBackups()?.toSet() ?: fail()