Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize URLs #534

Merged
merged 6 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion readium/lcp/src/main/java/org/readium/r2/lcp/LcpDecryptor.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import org.readium.r2.shared.util.Try
import org.readium.r2.shared.util.Url
import org.readium.r2.shared.util.data.ReadError
import org.readium.r2.shared.util.flatMap
import org.readium.r2.shared.util.getEquivalent
import org.readium.r2.shared.util.getOrElse
import org.readium.r2.shared.util.resource.FailureResource
import org.readium.r2.shared.util.resource.Resource
Expand All @@ -38,7 +39,7 @@ internal class LcpDecryptor(

fun transform(url: Url, resource: Resource): Resource {
return resource.flatMap {
val encryption = encryptionData[url]
val encryption = encryptionData.getEquivalent(url)

// Checks if the resource is encrypted and whether the encryption schemes of the resource
// and the DRM license are the same.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import org.readium.r2.lcp.service.DeviceService
import org.readium.r2.lcp.service.LcpClient
import org.readium.r2.lcp.service.LicensesRepository
import org.readium.r2.lcp.service.NetworkService
import org.readium.r2.shared.DelicateReadiumApi
import org.readium.r2.shared.InternalReadiumApi
import org.readium.r2.shared.extensions.tryOrNull
import org.readium.r2.shared.util.Instant
Expand Down Expand Up @@ -246,6 +247,7 @@ internal class License private constructor(
override val canReturnPublication: Boolean
get() = status?.link(StatusDocument.Rel.Return) != null

@OptIn(DelicateReadiumApi::class)
override suspend fun returnPublication(): Try<Unit, LcpError> {
try {
val status = this.documents.status
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV
val requestUrl = request.url.toUrl() ?: return false

// FIXME: I doubt this can work well. hasGesture considers itself unreliable.
return if (urlNotToOverrideLoading == requestUrl && request.hasGesture()) {
return if (urlNotToOverrideLoading?.isEquivalent(requestUrl) == true && request.hasGesture()) {
urlNotToOverrideLoading = null
false
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ public class EpubNavigatorFragment internal constructor(
val page = resources.withIndex().firstOrNull { (_, res) ->
when (res) {
is PageResource.EpubReflowable ->
res.link.url() == href
res.link.url().isEquivalent(href)
is PageResource.EpubFxl ->
res.leftUrl?.toString()?.endsWith(href.toString()) == true || res.rightUrl?.toString()?.endsWith(
href.toString()
Expand Down Expand Up @@ -785,7 +785,10 @@ public class EpubNavigatorFragment internal constructor(
paginationListener?.onPageLoaded()

val href = link.url()
if (state is State.Initializing || (state as? State.Loading)?.initialResourceHref == href) {
if (state is State.Initializing || (state as? State.Loading)?.initialResourceHref?.isEquivalent(
href
) == true
) {
state = State.Ready
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public fun Publication.normalizeLocator(locator: Locator): Locator {
?: return locator
)
} else { // Remote publication
// Check that the locator HREF relative to `self` exists int he manifest.
// Check that the locator HREF relative to `self` exists in the manifest.
val relativeHref = self.relativize(locator.href)
if (linkWithHref(relativeHref) != null) {
locator.copy(href = relativeHref)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import kotlinx.coroutines.flow.*
import org.readium.r2.navigator.extensions.let
import org.readium.r2.navigator.extensions.splitAt
import org.readium.r2.navigator.media.extensions.publicationId
import org.readium.r2.shared.DelicateReadiumApi
import org.readium.r2.shared.InternalReadiumApi
import org.readium.r2.shared.publication.Link
import org.readium.r2.shared.publication.Locator
Expand Down Expand Up @@ -141,6 +142,7 @@ public open class MediaService : MediaBrowserServiceCompat(), CoroutineScope by
private var notificationId: Int? = null
private var notification: Notification? = null

@OptIn(DelicateReadiumApi::class)
private val mediaPlayerListener = object : MediaPlayer.Listener {

/**
Expand Down Expand Up @@ -168,7 +170,7 @@ public open class MediaService : MediaBrowserServiceCompat(), CoroutineScope by
?.let { navigator.publication.linkWithHref(it) }
?.let { navigator.publication.locatorFromLink(it) }

if (locator != null && href != null && locator.href != href) {
if (locator != null && href != null && locator.href.isEquivalent(href)) {
Timber.e(
"Ambiguous playback location provided. HREF `$href` doesn't match locator $locator."
)
Expand Down
3 changes: 3 additions & 0 deletions readium/opds/src/main/java/org/readium/r2/opds/OPDS1Parser.kt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
package org.readium.r2.opds

import java.net.URL
import org.readium.r2.shared.DelicateReadiumApi
import org.readium.r2.shared.InternalReadiumApi
import org.readium.r2.shared.extensions.toList
import org.readium.r2.shared.extensions.toMap
Expand Down Expand Up @@ -101,6 +102,7 @@ public class OPDS1Parser {
public fun parse(xmlData: ByteArray, url: URL): ParseData =
throw NotImplementedError()

@OptIn(DelicateReadiumApi::class)
private fun parseFeed(root: ElementNode, url: Url): Feed {
val feedTitle = root.getFirst("title", Namespaces.Atom)?.text
?: throw Exception(OPDSParserError.MissingTitle.name)
Expand Down Expand Up @@ -273,6 +275,7 @@ public class OPDS1Parser {
}.mapFailure { ErrorException(it) }
}

@OptIn(DelicateReadiumApi::class)
private fun parseEntry(entry: ElementNode, baseUrl: Url): Publication? {
// A title is mandatory
val title = entry.getFirst("title", Namespaces.Atom)?.text
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ package org.readium.r2.shared

import java.io.Serializable
import org.readium.r2.shared.util.Url
import org.readium.r2.shared.util.isEquivalent

@InternalReadiumApi
public data class MediaOverlays(private val nodes: List<MediaOverlayNode> = listOf()) : Serializable {
Expand All @@ -21,11 +22,12 @@ public data class MediaOverlays(private val nodes: List<MediaOverlayNode> = list

private fun nodeForFragment(ref: Url?): MediaOverlayNode? = findNode(ref, this.nodes)

@OptIn(DelicateReadiumApi::class)
private fun findNode(ref: Url?, inNodes: List<MediaOverlayNode>): MediaOverlayNode? {
for (node in inNodes) {
if (node.role.contains("section")) {
return findNode(ref, node.children)
} else if (ref == null || node.text == ref) {
} else if (ref == null || node.text.isEquivalent(ref)) {
return node
}
}
Expand All @@ -36,6 +38,7 @@ public data class MediaOverlays(private val nodes: List<MediaOverlayNode> = list

private fun nodeAfterFragment(ref: Url?): MediaOverlayNode? = findNextNode(ref, this.nodes).found

@OptIn(DelicateReadiumApi::class)
private fun findNextNode(fragment: Url?, inNodes: List<MediaOverlayNode>): NextNodeResult {
var prevNodeFoundFlag = false
// For each node of the current scope...
Expand All @@ -55,7 +58,7 @@ public data class MediaOverlays(private val nodes: List<MediaOverlayNode> = list
prevNodeFoundFlag = ret.prevFound
}
// If the node text refer to filename or that filename is null, return node
else if (fragment == null || node.text == fragment) {
else if (fragment == null || node.text.isEquivalent(fragment)) {
prevNodeFoundFlag = true
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,13 +261,13 @@ public data class Link(
* Returns the first [Link] with the given [href], or null if not found.
*/
public fun List<Link>.indexOfFirstWithHref(href: Url): Int? =
indexOfFirst { it.url() == href }
indexOfFirst { it.url().isEquivalent(href) }
.takeUnless { it == -1 }

/**
* Finds the first link matching the given HREF.
*/
public fun List<Link>.firstWithHref(href: Url): Link? = firstOrNull { it.url() == href }
public fun List<Link>.firstWithHref(href: Url): Link? = firstOrNull { it.url().isEquivalent(href) }

/**
* Finds the first link with the given relation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ package org.readium.r2.shared.publication

import org.json.JSONArray
import org.json.JSONObject
import org.readium.r2.shared.DelicateReadiumApi
import org.readium.r2.shared.InternalReadiumApi
import org.readium.r2.shared.JSONable
import org.readium.r2.shared.extensions.optStringsFromArrayOrSingle
Expand Down Expand Up @@ -66,10 +67,11 @@ public data class Manifest(
* If there's no match, tries again after removing any query parameter and anchor from the
* given [href].
*/
@OptIn(DelicateReadiumApi::class)
public fun linkWithHref(href: Url): Link? {
fun List<Link>.deepLinkWithHref(href: Url): Link? {
for (l in this) {
if (l.url() == href) {
if (l.url().normalize() == href) {
return l
} else {
l.alternates.deepLinkWithHref(href)?.let { return it }
Expand All @@ -85,8 +87,9 @@ public data class Manifest(
?: links.deepLinkWithHref(href)
}

return find(href)
?: find(href.removeFragment().removeQuery())
val normalizedHref = href.normalize()
return find(normalizedHref)
?: find(normalizedHref.removeFragment().removeQuery())
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import org.jsoup.nodes.TextNode
import org.jsoup.parser.Parser
import org.jsoup.select.NodeTraversor
import org.jsoup.select.NodeVisitor
import org.readium.r2.shared.DelicateReadiumApi
import org.readium.r2.shared.ExperimentalReadiumApi
import org.readium.r2.shared.InternalReadiumApi
import org.readium.r2.shared.extensions.tryOrLog
Expand Down Expand Up @@ -279,6 +280,7 @@ public class HtmlResourceContentIterator internal constructor(
)
}

@OptIn(DelicateReadiumApi::class)
override fun head(node: Node, depth: Int) {
if (node is Element) {
val parent = ParentElement(node)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ private fun List<Link>.titleMatching(href: Url): String? {
}

private fun Link.titleMatching(targetHref: Url): String? {
if (url().removeFragment() == targetHref) {
if (url().removeFragment().isEquivalent(targetHref)) {
return title
}
return children.titleMatching(targetHref)
Expand Down
72 changes: 68 additions & 4 deletions readium/shared/src/main/java/org/readium/r2/shared/util/Url.kt
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,39 @@ public sealed class Url : Parcelable {
public open fun relativize(url: Url): Url =
checkNotNull(toURI().relativize(url.toURI()).toUrl())

/**
* Normalizes the URL using a subset of the RFC-3986 rules.
*
* https://datatracker.ietf.org/doc/html/rfc3986#section-6
*/
public open fun normalize(): Url =
uri.buildUpon()
.apply {
path?.let {
var normalizedPath = File(it).normalize().path
if (it.endsWith("/")) {
normalizedPath += "/"
}
path(normalizedPath)
}

if (this@Url is AbsoluteUrl) {
scheme(scheme.value)
}
}
.build()
.toUrl()!!

override fun toString(): String =
uri.toString()

/**
* Returns whether two URLs are strictly equal, by comparing their string representation.
*
* WARNING: Strict URL comparisons can be a source of bug, if the URLs are not normalized.
* In most cases, you should compare using [Url.isEquivalent].
*/
@DelicateReadiumApi
override fun equals(other: Any?): Boolean {
qnga marked this conversation as resolved.
Show resolved Hide resolved
if (this === other) return true
if (javaClass != other?.javaClass) return false
Expand All @@ -187,6 +217,15 @@ public sealed class Url : Parcelable {
return true
}

/**
* Returns whether the receiver is equivalent to the given `url` after normalization.
*/
@OptIn(DelicateReadiumApi::class)
public fun isEquivalent(url: Url?): Boolean {
url ?: return false
return normalize() == url.normalize()
}

override fun hashCode(): Int =
uri.toString().hashCode()

Expand Down Expand Up @@ -241,6 +280,9 @@ public class AbsoluteUrl private constructor(override val uri: Uri) : Url() {
public override fun resolve(url: Url): AbsoluteUrl =
super.resolve(url) as AbsoluteUrl

public override fun normalize(): AbsoluteUrl =
super.normalize() as AbsoluteUrl

/**
* Identifies the type of URL.
*/
Expand Down Expand Up @@ -294,6 +336,9 @@ public class RelativeUrl private constructor(override val uri: Uri) : Url() {
RelativeUrl(uri)
}
}

public override fun normalize(): RelativeUrl =
super.normalize() as RelativeUrl
}

/**
Expand All @@ -307,7 +352,7 @@ public class RelativeUrl private constructor(override val uri: Uri) : Url() {
*/
@DelicateReadiumApi
public fun Url.Companion.fromLegacyHref(href: String): Url? =
AbsoluteUrl(href) ?: Url.fromDecodedPath(href.removePrefix("/"))
AbsoluteUrl(href) ?: fromDecodedPath(href.removePrefix("/"))

/**
* According to the EPUB specification, the HREFs in the EPUB package must be valid URLs (so
Expand All @@ -318,9 +363,8 @@ public fun Url.Companion.fromLegacyHref(href: String): Url? =
* if we can't parse the URL.
*/
@InternalReadiumApi
public fun Url.Companion.fromEpubHref(href: String): Url? {
return (Url(href) ?: Url.fromDecodedPath(href))
}
public fun Url.Companion.fromEpubHref(href: String): Url? =
Url(href) ?: fromDecodedPath(href)

public fun File.toUrl(): AbsoluteUrl =
checkNotNull(AbsoluteUrl(Uri.fromFile(this)))
Expand Down Expand Up @@ -385,3 +429,23 @@ public value class FileExtension(
*/
public fun FileExtension?.appendToFilename(filename: String): String =
this?.let { "$filename.$value" } ?: filename

/**
* Returns whether the receiver is equivalent to the given `url` after normalization.
*/
@OptIn(DelicateReadiumApi::class)
public fun Url?.isEquivalent(url: Url?): Boolean {
if (this == null && url == null) return true
return this?.normalize() == url?.normalize()
}

/**
* Returns the value of the first key matching `key` after normalization.
*/
@OptIn(DelicateReadiumApi::class)
public fun <T> Map<Url, T>.getEquivalent(key: Url): T? =
get(key) ?: run {
val url = key.normalize()
keys.firstOrNull { it.normalize() == url }
?.let { get(it) }
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package org.readium.r2.shared.util.data

import org.readium.r2.shared.util.Try
import org.readium.r2.shared.util.Url
import org.readium.r2.shared.util.getEquivalent

internal class CachingReadable(
private val source: Readable
Expand Down Expand Up @@ -69,7 +70,7 @@ internal class CachingContainer(
mutableMapOf()

override fun get(url: Url): Readable? {
cache[url]?.let { return it }
cache.getEquivalent(url)?.let { return it }

val entry = container[url]
?: return null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

package org.readium.r2.shared.util.http

import org.readium.r2.shared.DelicateReadiumApi
import org.readium.r2.shared.util.AbsoluteUrl
import org.readium.r2.shared.util.Url
import org.readium.r2.shared.util.data.Container
Expand All @@ -27,6 +28,7 @@ public class HttpContainer(
private val client: HttpClient
) : Container<Resource> {

@OptIn(DelicateReadiumApi::class)
override fun get(url: Url): Resource? {
// We don't check that url matches any entry because that might save us from edge cases.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class SingleResourceContainer(
override val entries: Set<Url> = setOf(entryUrl)

override fun get(url: Url): Resource? {
if (url.removeFragment().removeQuery() != entryUrl) {
if (!url.removeFragment().removeQuery().isEquivalent(entryUrl)) {
return null
}

Expand Down
Loading
Loading