Improve Url equality

readium · Jun 11, 2024 · c1a50e9 · c1a50e9
1 parent c4eab48
commit c1a50e9
Show file tree

Hide file tree

Showing 3 changed files with 128 additions and 11 deletions.
diff --git a/readium/shared/src/main/java/org/readium/r2/shared/extensions/String.kt b/readium/shared/src/main/java/org/readium/r2/shared/extensions/String.kt
@@ -57,6 +57,12 @@ internal fun String.toJsonOrNull(): JSONObject? =
         null
     }
 
+/**
+ * Percent-decodes a string.
+ */
+internal fun String.percentDecoded(): String =
+    Uri.decode(this)
+
 /**
  * Percent-encodes an URL path section.
  *

diff --git a/readium/shared/src/main/java/org/readium/r2/shared/util/Url.kt b/readium/shared/src/main/java/org/readium/r2/shared/util/Url.kt
@@ -18,6 +18,7 @@ import kotlinx.parcelize.Parcelize
 import org.readium.r2.shared.DelicateReadiumApi
 import org.readium.r2.shared.InternalReadiumApi
 import org.readium.r2.shared.extensions.isPrintableAscii
+import org.readium.r2.shared.extensions.percentDecoded
 import org.readium.r2.shared.extensions.percentEncodedPath
 import org.readium.r2.shared.extensions.tryOrNull
 
@@ -176,17 +177,6 @@ public sealed class Url : Parcelable {
     override fun toString(): String =
         uri.toString()
 
-    override fun equals(other: Any?): Boolean {
-        if (this === other) return true
-        if (javaClass != other?.javaClass) return false
-
-        other as Url
-
-        if (uri.toString() != other.uri.toString()) return false
-
-        return true
-    }
-
     override fun hashCode(): Int =
         uri.toString().hashCode()
 
@@ -270,6 +260,19 @@ public class AbsoluteUrl private constructor(override val uri: Uri) : Url() {
      */
     public fun toFile(): File? =
         if (isFile) File(path!!) else null
+
+    override fun equals(other: Any?): Boolean {
+        if (this === other) return true
+        if (javaClass != other?.javaClass) return false
+
+        other as AbsoluteUrl
+
+        return scheme == other.scheme &&
+            uri.authority == other.uri.authority &&
+            path?.percentDecoded() == other.path?.percentDecoded() &&
+            query == other.query &&
+            fragment == other.fragment
+    }
 }
 
 /**
@@ -294,6 +297,17 @@ public class RelativeUrl private constructor(override val uri: Uri) : Url() {
                 RelativeUrl(uri)
             }
     }
+
+    override fun equals(other: Any?): Boolean {
+        if (this === other) return true
+        if (javaClass != other?.javaClass) return false
+
+        other as RelativeUrl
+
+        return path?.percentDecoded() == other.path?.percentDecoded() &&
+            query == other.query &&
+            fragment == other.fragment
+    }
 }
 
 /**

diff --git a/readium/shared/src/test/java/org/readium/r2/shared/util/UrlTest.kt b/readium/shared/src/test/java/org/readium/r2/shared/util/UrlTest.kt
@@ -7,6 +7,7 @@ import java.io.File
 import java.net.URI
 import java.net.URL
 import kotlin.test.assertIs
+import kotlin.test.assertNotEquals
 import kotlin.test.assertNotNull
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNull
@@ -399,4 +400,100 @@ class UrlTest {
         assertEquals(params.allNamed("empty"), emptyList<String>())
         assertEquals(params.allNamed("not-found"), emptyList<String>())
     }
+
+    @Test
+    fun equality() {
+        assertNotEquals(
+            AbsoluteUrl("http://example.com/foo/bar")!! as Url,
+            RelativeUrl("foo/bar") as Url
+        )
+
+        // Paths must be equal.
+        assertEquals(
+            AbsoluteUrl("http://example.com/foo/bar")!!,
+            AbsoluteUrl("http://example.com/foo/bar")
+        )
+        assertEquals(RelativeUrl("foo/bar")!!, RelativeUrl("foo/bar"))
+        assertNotEquals(
+            AbsoluteUrl("http://example.com/foo/bar")!!,
+            AbsoluteUrl("http://example.com/foo/baz")
+        )
+        assertNotEquals(RelativeUrl("foo/bar")!!, RelativeUrl("foo/baz"))
+
+        // Paths is compared percent and entity-decoded.
+        assertEquals(
+            AbsoluteUrl("http://example.com/c%27est%20valide")!!,
+            AbsoluteUrl("http://example.com/c%27est%20valide")
+        )
+        assertEquals(
+            AbsoluteUrl("http://example.com/c%27est%20valide")!!,
+            AbsoluteUrl("http://example.com/c'est%20valide")
+        )
+        assertEquals(RelativeUrl("c%27est%20valide")!!, RelativeUrl("c%27est%20valide"))
+        assertEquals(RelativeUrl("c%27est%20valide")!!, RelativeUrl("c'est%20valide"))
+
+        // Authority must be equal.
+        assertEquals(AbsoluteUrl("http://example.com/foo")!!, AbsoluteUrl("http://example.com/foo"))
+        assertNotEquals(
+            AbsoluteUrl("http://example.com/foo")!!,
+            AbsoluteUrl("http://example.com:80/foo")
+        )
+        assertNotEquals(
+            AbsoluteUrl("http://example.com:443/foo")!!,
+            AbsoluteUrl("http://example.com:80/foo")
+        )
+        assertNotEquals(
+            AbsoluteUrl("http://example.com/foo")!!,
+            AbsoluteUrl("http://example.com:80/foo")
+        )
+        assertNotEquals(
+            AbsoluteUrl("http://example.com/foo")!!,
+            AbsoluteUrl("http://domain.com/foo")
+        )
+        assertNotEquals(
+            AbsoluteUrl("http://example.com/foo")!!,
+            AbsoluteUrl("http://user:[email protected]/foo")
+        )
+        assertNotEquals(
+            AbsoluteUrl("http://other:[email protected]/foo")!!,
+            AbsoluteUrl("http://user:[email protected]/foo")
+        )
+
+        // Order of query parameters is important.
+        assertNotEquals(
+            AbsoluteUrl("http://example.com/foo/bar?a=a&b=b")!!,
+            AbsoluteUrl("http://example.com/foo/bar?b=b&a=a")
+        )
+        assertNotEquals(RelativeUrl("foo/bar?a=a&b=b")!!, RelativeUrl("foo/bar?b=b&a=a"))
+
+        // Content of parameters is important.
+        assertEquals(
+            AbsoluteUrl("http://example.com/foo/bar?a=a&b=b")!!,
+            AbsoluteUrl("http://example.com/foo/bar?a=a&b=b")
+        )
+        assertNotEquals(
+            AbsoluteUrl("http://example.com/foo/bar?a=a")!!,
+            AbsoluteUrl("http://example.com/foo/bar?b=b")
+        )
+        assertNotEquals(RelativeUrl("foo/bar?a=a")!!, RelativeUrl("foo/bar?b=b"))
+
+        // Scheme is case insensitive.
+        assertEquals(AbsoluteUrl("http://example.com/foo")!!, AbsoluteUrl("HTTP://example.com/foo"))
+        assertNotEquals(
+            AbsoluteUrl("http://example.com/foo")!!,
+            AbsoluteUrl("https://example.com/foo")
+        )
+
+        // Fragment is relevant.
+        assertEquals(
+            AbsoluteUrl("http://example.com/foo#fragment")!!,
+            AbsoluteUrl("http://example.com/foo#fragment")
+        )
+        assertEquals(RelativeUrl("foo/bar#fragment")!!, RelativeUrl("foo/bar#fragment"))
+        assertNotEquals(
+            AbsoluteUrl("http://example.com/foo#fragment")!!,
+            AbsoluteUrl("http://example.com/foo#other")
+        )
+        assertNotEquals(RelativeUrl("foo/bar#fragment")!!, RelativeUrl("foo/bar#other"))
+    }
 }