Extract Href Resolving Logic (#134)

- Move href resolving logic to a separate method `resolveHref`
- Add unit tests for `resolveHref` in `ResponseParserTest`

Author: rfc2822

src/main/kotlin/at/bitfire/dav4jvm/ktor/ResponseParser.kt

@@ -17,10 +17,13 @@ import at.bitfire.dav4jvm.property.webdav.ResourceType
 import at.bitfire.dav4jvm.property.webdav.WebDAV
 import io.ktor.http.HttpStatusCode
 import io.ktor.http.URLBuilder
+import io.ktor.http.URLParserException
 import io.ktor.http.Url
 import io.ktor.http.isSuccess
 import io.ktor.http.takeFrom
+import org.jetbrains.annotations.VisibleForTesting
 import org.xmlpull.v1.XmlPullParser
+import java.util.logging.Level
 import java.util.logging.Logger
 
 /**
@@ -60,38 +63,8 @@ class ResponseParser(
         while (!(eventType == XmlPullParser.END_TAG && parser.depth == depth)) {
             if (eventType == XmlPullParser.START_TAG && parser.depth == depth+1)
                 when (parser.propertyName()) {
-                    WebDAV.Href -> {
-                        var sHref = parser.nextText()
-                        var hierarchical = false
-                        if (!sHref.startsWith("/")) {
-                            /* According to RFC 4918 8.3 URL Handling, only absolute paths are allowed as relative
-                               URLs. However, some servers reply with relative paths. */
-                            val firstColon = sHref.indexOf(':')
-                            if (firstColon != -1) {
-                                /* There are some servers which return not only relative paths, but relative paths like "a:b.vcf",
-                                   which would be interpreted as scheme: "a", scheme-specific part: "b.vcf" normally.
-                                   For maximum compatibility, we prefix all relative paths which contain ":" (but not "://"),
-                                   with "./" to allow resolving by HttpUrl. */
-                                try {
-                                    if (sHref.substring(firstColon, firstColon + 3) == "://")
-                                        hierarchical = true
-                                } catch (e: IndexOutOfBoundsException) {
-                                    // no "://"
-                                }
-                                if (!hierarchical)
-                                    sHref = "./$sHref"
-
-                            }
-                        }
-
-                        if (!hierarchical) {
-                            val urlBuilder = URLBuilder(location).takeFrom(sHref)
-                            urlBuilder.pathSegments = urlBuilder.pathSegments.filterNot { it == "." } // Drop segments that are "./"
-                            hrefOrNull = urlBuilder.build()
-                        } else {
-                            hrefOrNull = URLBuilder(location).takeFrom(sHref).build()
-                        }
-                    }
+                    WebDAV.Href ->
+                        hrefOrNull = resolveHref(parser.nextText())
                     WebDAV.Status ->
                         status = KtorHttpUtils.parseStatusLine(parser.nextText())
                     WebDAV.PropStat ->
@@ -168,4 +141,42 @@ class ResponseParser(
         )
     }
 
+    @VisibleForTesting
+    internal fun resolveHref(hrefString: String): Url? {
+        var sHref = hrefString
+
+        var preserve = false
+        if (!sHref.startsWith("/")) {
+            /* According to RFC 4918 8.3 URL Handling, only absolute paths are allowed as relative
+               URLs. However, some servers reply with relative paths. */
+            val firstColon = sHref.indexOf(':')
+            if (firstColon != -1) {
+                /* There are some servers which return not only relative paths, but relative paths like "a:b.vcf",
+                   which would be interpreted as scheme: "a", scheme-specific part: "b.vcf" normally.
+                   For maximum compatibility, we prefix all relative paths which contain ":" (but not "://"),
+                   with "./" to allow resolving by HttpUrl. */
+                try {
+                    if (sHref.substring(firstColon, firstColon + 3) == "://")
+                        preserve = true
+                } catch (_: IndexOutOfBoundsException) {
+                    // no "://"
+                }
+                if (!preserve)
+                    sHref = "./$sHref"
+            }
+        }
+
+        val urlBuilder = try {
+            URLBuilder(location).takeFrom(sHref)
+        } catch (e: URLParserException) {
+            logger.log(Level.WARNING, "Unresolvable <href> in <response>: $hrefString", e)
+            return null
+        }
+
+        if (!preserve)      // drop segments that are "./"
+            urlBuilder.pathSegments = urlBuilder.pathSegments.filterNot { it == "." }
+
+        return urlBuilder.build()
+    }
+
 }

src/test/kotlin/at/bitfire/dav4jvm/ktor/ResponseParserTest.kt

@@ -0,0 +1,56 @@
+/*
+ * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * SPDX-License-Identifier: MPL-2.0
+ */
+
+package at.bitfire.dav4jvm.ktor
+
+import io.ktor.http.Url
+import org.junit.Assert.assertEquals
+import org.junit.Test
+
+class ResponseParserTest {
+
+    val baseUrl = Url("https://example.com/collection/")
+    val parser = ResponseParser(baseUrl, callback = { _, _ ->
+        // no-op
+    })
+
+    @Test
+    fun `resolveHref with absolute URL`() {
+        assertEquals(
+            Url("https://example.com/collection/member"),
+            parser.resolveHref("https://example.com/collection/member")
+        )
+    }
+
+    @Test
+    fun `resolveHref with absolute path`() {
+        assertEquals(
+            Url("https://example.com/collection/member"),
+            parser.resolveHref("/collection/member")
+        )
+    }
+
+    @Test
+    fun `resolveHref with relative path`() {
+        assertEquals(
+            Url("https://example.com/collection/member"),
+            parser.resolveHref("member")
+        )
+    }
+
+    @Test
+    fun `resolveHref with relative path with colon`() {
+        assertEquals(
+            Url("https://example.com/collection/mem:ber"),
+            parser.resolveHref("mem:ber")
+        )
+    }
+
+}