From c1bc14348831bcdb00f3a6eec4859b81c7dc3728 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Thu, 8 Aug 2024 13:24:53 +0200 Subject: [PATCH] Simplify UrlUtils.equals(), move to extension (#54) --- .../kotlin/at/bitfire/dav4jvm/Response.kt | 30 ++++--- .../kotlin/at/bitfire/dav4jvm/UrlUtils.kt | 85 ++++++++----------- .../kotlin/at/bitfire/dav4jvm/UrlUtilsTest.kt | 35 ++++---- 3 files changed, 71 insertions(+), 79 deletions(-) diff --git a/src/main/kotlin/at/bitfire/dav4jvm/Response.kt b/src/main/kotlin/at/bitfire/dav4jvm/Response.kt index 0a0049c..4854746 100644 --- a/src/main/kotlin/at/bitfire/dav4jvm/Response.kt +++ b/src/main/kotlin/at/bitfire/dav4jvm/Response.kt @@ -115,7 +115,7 @@ data class Response( val depth = parser.depth - var href: HttpUrl? = null + var hrefOrNull: HttpUrl? = null var status: StatusLine? = null val propStat = mutableListOf() var error: List? = null @@ -147,7 +147,7 @@ data class Response( sHref = "./$sHref" } } - href = location.resolve(sHref) + hrefOrNull = location.resolve(sHref) } STATUS -> status = try { @@ -166,10 +166,11 @@ data class Response( eventType = parser.next() } - if (href == null) { + if (hrefOrNull == null) { logger.warning("Ignoring XML response element without valid href") return } + var href: HttpUrl = hrefOrNull // guaranteed to be not null // if we know this resource is a collection, make sure href has a trailing slash // (for clarity and resolving relative paths) @@ -179,23 +180,24 @@ data class Response( .firstOrNull() ?.let { type -> if (type.types.contains(ResourceType.COLLECTION)) - href = UrlUtils.withTrailingSlash(href!!) + href = UrlUtils.withTrailingSlash(href) } //log.log(Level.FINE, "Received properties for $href", if (status != null) status else propStat) // Which resource does this represent? val relation = when { - UrlUtils.equals(UrlUtils.omitTrailingSlash(href!!), UrlUtils.omitTrailingSlash(location)) -> + UrlUtils.omitTrailingSlash(href).equalsForWebDAV(UrlUtils.omitTrailingSlash(location)) -> HrefRelation.SELF + else -> { - if (location.scheme == href!!.scheme && location.host == href!!.host && location.port == href!!.port) { + if (location.scheme == href.scheme && location.host == href.host && location.port == href.port) { val locationSegments = location.pathSegments - val hrefSegments = href!!.pathSegments + val hrefSegments = href.pathSegments // don't compare trailing slash segment ("") var nBasePathSegments = locationSegments.size - if (locationSegments[nBasePathSegments-1] == "") + if (locationSegments[nBasePathSegments - 1] == "") nBasePathSegments-- /* example: locationSegments = [ "davCollection", "" ] @@ -217,12 +219,12 @@ data class Response( callback.onResponse( Response( - location, - href!!, - status, - propStat, - error, - newLocation + requestedUrl = location, + href = href, + status = status, + propstat = propStat, + error = error, + newLocation = newLocation ), relation ) diff --git a/src/main/kotlin/at/bitfire/dav4jvm/UrlUtils.kt b/src/main/kotlin/at/bitfire/dav4jvm/UrlUtils.kt index d2995b7..84876bf 100644 --- a/src/main/kotlin/at/bitfire/dav4jvm/UrlUtils.kt +++ b/src/main/kotlin/at/bitfire/dav4jvm/UrlUtils.kt @@ -6,58 +6,14 @@ package at.bitfire.dav4jvm +import at.bitfire.dav4jvm.UrlUtils.omitTrailingSlash +import at.bitfire.dav4jvm.UrlUtils.withTrailingSlash import okhttp3.HttpUrl -import java.net.URI -import java.net.URISyntaxException -import java.util.logging.Level -import java.util.logging.Logger object UrlUtils { - private val logger - get() = Logger.getLogger(javaClass.name) - - - /** - * Compares two URLs in WebDAV context. If two URLs are considered *equal*, both - * represent the same WebDAV resource (e.g. `http://host:80/folder1` and `http://HOST/folder1#somefragment`). - * - * It decodes %xx entities in the path, so `/my@dav` and `/my%40dav` are considered the same. - * This is important to process multi-status responses: some servers serve a multi-status - * response with href `/my@dav` when you request `/my%40dav` and vice versa. - * - * This method does not deal with trailing slashes, so if you want to compare collection URLs, - * make sure they both (don't) have a trailing slash before calling this method, for instance - * with [omitTrailingSlash] or [withTrailingSlash]. - * - * @param url1 the first URL to be compared - * @param url2 the second URL to be compared - * - * @return whether [url1] and [url2] (usually) represent the same WebDAV resource - */ - fun equals(url1: HttpUrl, url2: HttpUrl): Boolean { - // if okhttp thinks the two URLs are equal, they're in any case - // (and it's a simple String comparison) - if (url1 == url2) - return true - - // convert to java.net.URI (also corrects some mistakes) - val uri1 = url1.toUri() - val uri2 = url2.toUri() - - // if the URIs are the same (ignoring scheme case and fragments), they're equal for us - if (uri1.scheme.equals(uri2.scheme, true) && uri1.schemeSpecificPart == uri2.schemeSpecificPart) - return true - - return try { - val decoded1 = URI(url1.scheme, uri1.schemeSpecificPart, null) - val decoded2 = URI(uri2.scheme, uri2.schemeSpecificPart, null) - decoded1 == decoded2 - } catch (e: URISyntaxException) { - logger.log(Level.WARNING, "Couldn't decode URI for comparison, assuming inequality", e) - false - } - } + @Deprecated("Use equalsForWebDAV instead", ReplaceWith("url1.equalsForWebDAV(url2)")) + fun equals(url1: HttpUrl, url2: HttpUrl) = url1.equalsForWebDAV(url2) /** * Gets the first-level domain name (without subdomains) from a host name. @@ -117,4 +73,37 @@ object UrlUtils { url.newBuilder().addPathSegment("").build() } +} + +/** + * Compares two [HttpUrl]s in WebDAV context. If two URLs are considered *equal*, both + * represent the same WebDAV resource. + * + * The fragment of an URL is ignored, e.g. `http://host:80/folder1` and `http://HOST/folder1#somefragment` + * are considered to be equal. + * + * [HttpUrl] is less strict than [java.net.URI] and allows for instance (not encoded) square brackets in the path. + * So this method tries to normalize the URI by converting it to a [java.net.URI] (encodes for instance square brackets) + * and then comparing the scheme and scheme-specific part (without fragment). + * + * Attention: **This method does not deal with trailing slashes**, so if you want to compare collection URLs, + * make sure they both (don't) have a trailing slash before calling this method, for instance + * with [omitTrailingSlash] or [withTrailingSlash]. + * + * @param other the URL to compare the current object with + * + * @return whether the URLs are considered to represent the same WebDAV resource + */ +fun HttpUrl.equalsForWebDAV(other: HttpUrl): Boolean { + // if okhttp thinks the two URLs are equal, they're in any case + // (and it's a simple String comparison) + if (this == other) + return true + + // convert to java.net.URI (also corrects some mistakes and escapes for instance square brackets) + val uri1 = toUri() + val uri2 = other.toUri() + + // if the URIs are the same (ignoring scheme case and fragments), they're equal for us + return uri1.scheme.equals(uri2.scheme, true) && uri1.schemeSpecificPart == uri2.schemeSpecificPart } \ No newline at end of file diff --git a/src/test/kotlin/at/bitfire/dav4jvm/UrlUtilsTest.kt b/src/test/kotlin/at/bitfire/dav4jvm/UrlUtilsTest.kt index c58e2f8..4816481 100644 --- a/src/test/kotlin/at/bitfire/dav4jvm/UrlUtilsTest.kt +++ b/src/test/kotlin/at/bitfire/dav4jvm/UrlUtilsTest.kt @@ -15,22 +15,6 @@ import org.junit.Test class UrlUtilsTest { - @Test - fun testEquals() { - assertTrue(UrlUtils.equals("http://host/resource".toHttpUrl(), "http://host/resource".toHttpUrl())) - assertTrue(UrlUtils.equals("http://host:80/resource".toHttpUrl(), "http://host/resource".toHttpUrl())) - assertTrue(UrlUtils.equals("https://HOST:443/resource".toHttpUrl(), "https://host/resource".toHttpUrl())) - assertTrue(UrlUtils.equals("https://host:443/my@dav/".toHttpUrl(), "https://host/my%40dav/".toHttpUrl())) - assertTrue(UrlUtils.equals("http://host/resource".toHttpUrl(), "http://host/resource#frag1".toHttpUrl())) - - assertFalse(UrlUtils.equals("http://host/resource".toHttpUrl(), "http://host/resource/".toHttpUrl())) - assertFalse(UrlUtils.equals("http://host/resource".toHttpUrl(), "http://host:81/resource".toHttpUrl())) - - assertTrue(UrlUtils.equals("https://www.example.com/folder/[X]Y!.txt".toHttpUrl(), "https://www.example.com/folder/[X]Y!.txt".toHttpUrl())) - assertTrue(UrlUtils.equals("https://www.example.com/folder/%5BX%5DY!.txt".toHttpUrl(), "https://www.example.com/folder/[X]Y!.txt".toHttpUrl())) - assertTrue(UrlUtils.equals("https://www.example.com/folder/%5bX%5dY%21.txt".toHttpUrl(), "https://www.example.com/folder/[X]Y!.txt".toHttpUrl())) - } - @Test fun testHostToDomain() { assertNull(UrlUtils.hostToDomain(null)) @@ -59,4 +43,21 @@ class UrlUtilsTest { assertEquals("http://host/resource/".toHttpUrl(), UrlUtils.withTrailingSlash("http://host/resource/".toHttpUrl())) } -} + + @Test + fun testHttpUrl_EqualsForWebDAV() { + assertTrue("http://host/resource".toHttpUrl().equalsForWebDAV("http://host/resource".toHttpUrl())) + assertTrue("http://host:80/resource".toHttpUrl().equalsForWebDAV("http://host/resource".toHttpUrl())) + assertTrue("https://HOST:443/resource".toHttpUrl().equalsForWebDAV("https://host/resource".toHttpUrl())) + assertTrue("https://host:443/my@dav/".toHttpUrl().equalsForWebDAV("https://host/my%40dav/".toHttpUrl())) + assertTrue("http://host/resource".toHttpUrl().equalsForWebDAV("http://host/resource#frag1".toHttpUrl())) + + assertFalse("http://host/resource".toHttpUrl().equalsForWebDAV("http://host/resource/".toHttpUrl())) + assertFalse("http://host/resource".toHttpUrl().equalsForWebDAV("http://host:81/resource".toHttpUrl())) + + assertTrue("https://www.example.com/folder/[X]Y!.txt".toHttpUrl().equalsForWebDAV("https://www.example.com/folder/[X]Y!.txt".toHttpUrl())) + assertTrue("https://www.example.com/folder/%5BX%5DY!.txt".toHttpUrl().equalsForWebDAV("https://www.example.com/folder/[X]Y!.txt".toHttpUrl())) + assertTrue("https://www.example.com/folder/%5bX%5dY%21.txt".toHttpUrl().equalsForWebDAV("https://www.example.com/folder/[X]Y!.txt".toHttpUrl())) + } + +} \ No newline at end of file