Validate Link's href when creating a URL

Sources/Navigator/Audiobook/PublicationMediaLoader.swift

@@ -35,8 +35,10 @@ final class PublicationMediaLoader: NSObject, AVAssetResourceLoaderDelegate {
 
     /// Creates a new `AVURLAsset` to serve the given `link`.
     func makeAsset(for link: Link) throws -> AVURLAsset {
-        let originalURL = link.url(relativeTo: publication.baseURL)
-        guard var components = URLComponents(url: originalURL.url, resolvingAgainstBaseURL: true) else {
+        guard
+            let originalURL = try? link.url(relativeTo: publication.baseURL),
+            var components = URLComponents(url: originalURL.url, resolvingAgainstBaseURL: true)
+        else {
             throw AssetError.invalidHREF(link.href)
         }
 

Sources/Navigator/CBZ/CBZNavigatorViewController.swift

@@ -169,11 +169,13 @@ open class CBZNavigatorViewController: UIViewController, VisualNavigator, Loggab
     }
 
     private func imageViewController(at index: Int) -> ImageViewController? {
-        guard publication.readingOrder.indices.contains(index) else {
+        guard
+            publication.readingOrder.indices.contains(index),
+            let url = try? publication.readingOrder[index].url(relativeTo: publicationBaseURL)
+        else {
             return nil
         }
 
-        let url = publication.readingOrder[index].url(relativeTo: publicationBaseURL)
         return ImageViewController(index: index, url: url.url)
     }
 

Sources/Navigator/EPUB/EPUBNavigatorViewModel.swift

@@ -176,7 +176,7 @@ final class EPUBNavigatorViewModel: Loggable {
     }
 
     func url(to link: Link) -> AnyURL? {
-        link.url(relativeTo: publicationBaseURL)
+        try? link.url(relativeTo: publicationBaseURL)
     }
 
     private func serveFile(at file: FileURL, baseEndpoint: HTTPServerEndpoint) throws -> HTTPURL {

Sources/Navigator/EPUB/EPUBSpread.swift

@@ -83,7 +83,10 @@ struct EPUBSpread: Loggable {
     ///   - page [left|center|right]: (optional) Page position of the linked resource in the spread.
     func json(forBaseURL baseURL: HTTPURL) -> [[String: Any]] {
         func makeLinkJSON(_ link: Link, page: Presentation.Page? = nil) -> [String: Any]? {
-            let url = link.url(relativeTo: baseURL)
+            guard let url = try? link.url(relativeTo: baseURL) else {
+                return nil
+            }
+
             let page = page ?? link.properties.page ?? readingProgression.leadingPage
             return [
                 "link": link.json,

Sources/OPDS/OPDS2Parser.swift

@@ -180,7 +180,7 @@ public class OPDS2Parser: Loggable {
                     }
                     for linkDict in links {
                         var link = try Link(json: linkDict)
-                        link.normalizeHREFs(to: feedURL)
+                        try link.normalizeHREFs(to: feedURL)
                         facet.links.append(link)
                     }
                 }
@@ -192,7 +192,7 @@ public class OPDS2Parser: Loggable {
     static func parseLinks(feed: Feed, feedURL: URL, links: [[String: Any]]) throws {
         for linkDict in links {
             var link = try Link(json: linkDict)
-            link.normalizeHREFs(to: feedURL)
+            try link.normalizeHREFs(to: feedURL)
             feed.links.append(link)
         }
     }
@@ -207,7 +207,7 @@ public class OPDS2Parser: Loggable {
     static func parseNavigation(feed: Feed, feedURL: URL, navLinks: [[String: Any]]) throws {
         for navDict in navLinks {
             var link = try Link(json: navDict)
-            link.normalizeHREFs(to: feedURL)
+            try link.normalizeHREFs(to: feedURL)
             feed.navigation.append(link)
         }
     }
@@ -233,7 +233,7 @@ public class OPDS2Parser: Loggable {
                     }
                     for linkDict in links {
                         var link = try Link(json: linkDict)
-                        link.normalizeHREFs(to: feedURL)
+                        try link.normalizeHREFs(to: feedURL)
                         group.links.append(link)
                     }
                 case "navigation":
@@ -242,7 +242,7 @@ public class OPDS2Parser: Loggable {
                     }
                     for linkDict in links {
                         var link = try Link(json: linkDict)
-                        link.normalizeHREFs(to: feedURL)
+                        try link.normalizeHREFs(to: feedURL)
                         group.navigation.append(link)
                     }
                 case "publications":

Sources/Shared/Fetcher/ArchiveFetcher.swift

@@ -28,7 +28,7 @@ public final class ArchiveFetcher: Fetcher, Loggable {
 
     public func get(_ link: Link) -> Resource {
         guard
-            let path = link.url().relativeURL?.path,
+            let path = try? link.url().relativeURL?.path,
             let entry = findEntry(at: path),
             let reader = archive.readEntry(at: entry.path)
         else {

Sources/Shared/Fetcher/FileFetcher.swift

@@ -23,7 +23,7 @@ public final class FileFetcher: Fetcher, Loggable {
     }
 
     public func get(_ link: Link) -> Resource {
-        if let linkHREF = link.url().relativeURL {
+        if let linkHREF = try? link.url().relativeURL {
             for (href, url) in paths {
                 if linkHREF == href {
                     return FileResource(link: link, file: url)

Sources/Shared/Fetcher/HTTPFetcher.swift

@@ -21,7 +21,7 @@ public final class HTTPFetcher: Fetcher, Loggable {
     public let links: [Link] = []
 
     public func get(_ link: Link) -> Resource {
-        guard let url = link.url(relativeTo: baseURL).httpURL else {
+        guard let url = try? link.url(relativeTo: baseURL).httpURL else {
             log(.error, "Not a valid HTTP URL: \(link.href)")
             return FailureResource(link: link, error: .badRequest(HTTPError(kind: .malformedRequest(url: link.href))))
         }

Sources/Shared/Publication/HREFNormalizer.swift

@@ -8,27 +8,27 @@ import Foundation
 
 public extension Manifest {
     /// Resolves the HREFs in the ``Manifest`` to the link with `rel="self"`.
-    mutating func normalizeHREFsToSelf() {
-        guard let base = link(withRel: .self)?.url() else {
+    mutating func normalizeHREFsToSelf() throws {
+        guard let base = try link(withRel: .self)?.url() else {
             return
         }
 
-        normalizeHREFs(to: base)
+        try normalizeHREFs(to: base)
     }
 
     /// Resolves the HREFs in the ``Manifest`` to the given `baseURL`.
-    mutating func normalizeHREFs<T: URLConvertible>(to baseURL: T) {
-        transform(HREFNormalizer(baseURL: baseURL))
+    mutating func normalizeHREFs<T: URLConvertible>(to baseURL: T) throws {
+        try transform(HREFNormalizer(baseURL: baseURL))
     }
 }
 
 public extension Link {
     /// Resolves the HREFs in the ``Link`` to the given `baseURL`.
-    mutating func normalizeHREFs<T: URLConvertible>(to baseURL: T?) {
+    mutating func normalizeHREFs<T: URLConvertible>(to baseURL: T?) throws {
         guard let baseURL = baseURL else {
             return
         }
-        transform(HREFNormalizer(baseURL: baseURL))
+        try transform(HREFNormalizer(baseURL: baseURL))
     }
 }
 
@@ -39,12 +39,12 @@ private struct HREFNormalizer: ManifestTransformer, Loggable {
         self.baseURL = baseURL.anyURL
     }
 
-    func transform(link: inout Link) {
+    func transform(link: inout Link) throws {
         guard !link.templated else {
             log(.warning, "Cannot safely resolve a URI template to a base URL before expanding it: \(link.href)")
             return
         }
 
-        link.href = link.url(relativeTo: baseURL).string
+        link.href = try link.url(relativeTo: baseURL).string
     }
 }

Sources/Shared/Publication/Link.swift

@@ -7,6 +7,11 @@
 import Foundation
 import ReadiumInternal
 
+public enum LinkError: Error, Equatable {
+    /// The link's HREF is not a valid URL.
+    case invalidHREF(String)
+}
+
 /// Link Object for the Readium Web Publication Manifest.
 /// https://readium.org/webpub-manifest/schema/link.schema.json
 public struct Link: JSONEquatable, Hashable {
@@ -91,15 +96,29 @@ public struct Link: JSONEquatable, Hashable {
         warnings: WarningLogger? = nil
     ) throws {
         guard let jsonObject = json as? [String: Any],
-              let href = jsonObject["href"] as? String
+              var href = jsonObject["href"] as? String
         else {
             warnings?.log("`href` is required", model: Self.self, source: json)
             throw JSONError.parsing(Self.self)
         }
+
+        let templated = (jsonObject["templated"] as? Bool) ?? false
+
+        // We support existing publications with incorrect HREFs (not valid percent-encoded
+        // URIs). We try to parse them first as valid, but fall back on a percent-decoded
+        // path if it fails.
+        if AnyURL(string: href) == nil {
+            warnings?.log("`href` is not a valid percent-encoded URL", model: Self.self, source: json)
+            guard let url = RelativeURL(path: href) else {
+                throw JSONError.parsing(Self.self)
+            }
+            href = url.string
+        }
+
         self.init(
             href: href,
             type: jsonObject["type"] as? String,
-            templated: (jsonObject["templated"] as? Bool) ?? false,
+            templated: templated,
             title: jsonObject["title"] as? String,
             rels: .init(json: jsonObject["rel"]),
             properties: (try? Properties(json: jsonObject["properties"], warnings: warnings)) ?? Properties(),
@@ -145,15 +164,19 @@ public struct Link: JSONEquatable, Hashable {
     /// according to RFC 6570.
     public func url(
         parameters: [String: LosslessStringConvertible] = [:]
-    ) -> AnyURL {
+    ) throws -> AnyURL {
         var href = href
         if templated {
             href = URITemplate(href).expand(with: parameters)
         }
         if href.isEmpty {
             href = "#"
         }
-        return AnyURL(string: href)!
+
+        guard let url = AnyURL(string: href) else {
+            throw LinkError.invalidHREF(href)
+        }
+        return url
     }
 
     /// Returns the URL represented by this link's HREF, resolved to the given
@@ -164,8 +187,8 @@ public struct Link: JSONEquatable, Hashable {
     public func url<T: URLConvertible>(
         relativeTo baseURL: T?,
         parameters: [String: LosslessStringConvertible] = [:]
-    ) -> AnyURL {
-        let url = url(parameters: parameters)
+    ) throws -> AnyURL {
+        let url = try url(parameters: parameters)
         return baseURL?.anyURL.resolve(url) ?? url
     }
 

Sources/Shared/Publication/ManifestTransformer.swift

@@ -8,101 +8,101 @@ import Foundation
 
 /// Transforms a ``Manifest``'s components.
 public protocol ManifestTransformer {
-    func transform(manifest: inout Manifest)
-    func transform(metadata: inout Metadata)
-    func transform(link: inout Link)
+    func transform(manifest: inout Manifest) throws
+    func transform(metadata: inout Metadata) throws
+    func transform(link: inout Link) throws
 }
 
 public protocol ManifestTransformable {
-    mutating func transform(_ transformer: ManifestTransformer)
+    mutating func transform(_ transformer: ManifestTransformer) throws
 }
 
 public extension ManifestTransformer {
-    func transform(manifest: inout Manifest) {}
-    func transform(metadata: inout Metadata) {}
-    func transform(link: inout Link) {}
+    func transform(manifest: inout Manifest) throws {}
+    func transform(metadata: inout Metadata) throws {}
+    func transform(link: inout Link) throws {}
 }
 
 extension Manifest: ManifestTransformable {
     /// Transforms the receiver ``Manifest``, applying the given `transformer`
     /// to each component.
-    public mutating func transform(_ transformer: ManifestTransformer) {
-        metadata.transform(transformer)
-        links.transform(transformer)
-        readingOrder.transform(transformer)
-        resources.transform(transformer)
-        tableOfContents.transform(transformer)
-        subcollections.transform(transformer)
+    public mutating func transform(_ transformer: ManifestTransformer) throws {
+        try metadata.transform(transformer)
+        try links.transform(transformer)
+        try readingOrder.transform(transformer)
+        try resources.transform(transformer)
+        try tableOfContents.transform(transformer)
+        try subcollections.transform(transformer)
 
-        transformer.transform(manifest: &self)
+        try transformer.transform(manifest: &self)
     }
 }
 
 extension Metadata: ManifestTransformable {
-    public mutating func transform(_ transformer: ManifestTransformer) {
-        subjects.transform(transformer)
-        authors.transform(transformer)
-        translators.transform(transformer)
-        editors.transform(transformer)
-        artists.transform(transformer)
-        illustrators.transform(transformer)
-        letterers.transform(transformer)
-        pencilers.transform(transformer)
-        colorists.transform(transformer)
-        inkers.transform(transformer)
-        narrators.transform(transformer)
-        contributors.transform(transformer)
-        publishers.transform(transformer)
-        imprints.transform(transformer)
-        belongsTo.transform(transformer)
+    public mutating func transform(_ transformer: ManifestTransformer) throws {
+        try subjects.transform(transformer)
+        try authors.transform(transformer)
+        try translators.transform(transformer)
+        try editors.transform(transformer)
+        try artists.transform(transformer)
+        try illustrators.transform(transformer)
+        try letterers.transform(transformer)
+        try pencilers.transform(transformer)
+        try colorists.transform(transformer)
+        try inkers.transform(transformer)
+        try narrators.transform(transformer)
+        try contributors.transform(transformer)
+        try publishers.transform(transformer)
+        try imprints.transform(transformer)
+        try belongsTo.transform(transformer)
 
-        transformer.transform(metadata: &self)
+        try transformer.transform(metadata: &self)
     }
 }
 
 extension PublicationCollection: ManifestTransformable {
-    public mutating func transform(_ transformer: ManifestTransformer) {
-        links.transform(transformer)
-        subcollections.transform(transformer)
+    public mutating func transform(_ transformer: ManifestTransformer) throws {
+        try links.transform(transformer)
+        try subcollections.transform(transformer)
     }
 }
 
 extension Contributor: ManifestTransformable {
-    public mutating func transform(_ transformer: ManifestTransformer) {
-        links.transform(transformer)
+    public mutating func transform(_ transformer: ManifestTransformer) throws {
+        try links.transform(transformer)
     }
 }
 
 extension Subject: ManifestTransformable {
-    public mutating func transform(_ transformer: ManifestTransformer) {
-        links.transform(transformer)
+    public mutating func transform(_ transformer: ManifestTransformer) throws {
+        try links.transform(transformer)
     }
 }
 
 extension Link: ManifestTransformable {
-    public mutating func transform(_ transformer: ManifestTransformer) {
-        alternates.transform(transformer)
-        children.transform(transformer)
+    public mutating func transform(_ transformer: ManifestTransformer) throws {
+        try alternates.transform(transformer)
+        try children.transform(transformer)
 
-        transformer.transform(link: &self)
+        try transformer.transform(link: &self)
     }
 }
 
 extension Array: ManifestTransformable where Element: ManifestTransformable {
-    public mutating func transform(_ transformer: ManifestTransformer) {
-        self = map {
+    public mutating func transform(_ transformer: ManifestTransformer) throws {
+        self = try map {
             var copy = $0
-            copy.transform(transformer)
+            try copy.transform(transformer)
             return copy
         }
     }
 }
 
 extension Dictionary: ManifestTransformable where Value: ManifestTransformable {
-    public mutating func transform(_ transformer: ManifestTransformer) {
-        self = mapValues {
+    public mutating func transform(_ transformer: ManifestTransformer) throws {
+        self = try mapValues {
             var copy = $0
-            copy.transform(transformer)
+            try copy.transform(transformer)
             return copy
         }
     }