fix: normalize URLs before checking if the resources exist

rdeltour · rdeltour · commit 03236685b6bf · 2023-04-22T10:53:03.000+02:00
URLs were not normalized before performing existence checks. So percent-encoded URLs sometimes triggered `RSC-001` or `RSC-007` errors. This commit introduces a new `normalize(URL)` method in the `URLUtils` class. Normalization is now used when checking a URL. This notably applies to resource and ID existence checks. Important Note: URL normalization is not well-defined. Some percent-encoding normalization is described in RFC3986, but is not defined in the URL standard. Also, normalization (as useful for EPUBCheck) is also dependent on the URL scheme. The normalization we apply is quite naïve and might need to be improved in the future. It should however cover the majority of HTTP URL real-world scenarios. Fix #1479
diff --git a/src/main/java/com/adobe/epubcheck/ocf/OCFContainer.java b/src/main/java/com/adobe/epubcheck/ocf/OCFContainer.java
@@ -79,7 +79,14 @@ public OCFContainer(Builder builder)
 
   public boolean contains(URL resource)
   {
-    return resources.containsKey(resource);
+    if (resources.containsKey(resource))
+    {
+      return true;
+    }
+    else
+    {
+      return resources.containsKey(URLUtils.normalize(resource));
+    }
   }
 
   @Override
@@ -134,5 +141,4 @@ public boolean isRemote(URL url)
     }
   }
 
-
 }
diff --git a/src/main/java/org/w3c/epubcheck/core/references/URLChecker.java b/src/main/java/org/w3c/epubcheck/core/references/URLChecker.java
@@ -2,6 +2,8 @@
 
 import java.net.URI;
 
+import org.w3c.epubcheck.util.url.URLUtils;
+
 import com.adobe.epubcheck.api.EPUBLocation;
 import com.adobe.epubcheck.api.Report;
 import com.adobe.epubcheck.messages.MessageId;
@@ -64,7 +66,7 @@ public URL setBase(String newBase, EPUBLocation location)
 
   public URL checkURL(String string, EPUBLocation location)
   {
-    URL url = resolveURL(string, false, location);
+    URL url = URLUtils.normalize(resolveURL(string, false, location));
     return url;
   }
 
diff --git a/src/main/java/org/w3c/epubcheck/util/url/URLUtils.java b/src/main/java/org/w3c/epubcheck/util/url/URLUtils.java
@@ -14,6 +14,7 @@
 import io.mola.galimatias.GalimatiasParseException;
 import io.mola.galimatias.ParseIssue;
 import io.mola.galimatias.URL;
+import io.mola.galimatias.canonicalize.DecodeUnreservedCanonicalizer;
 
 //FIXME 2022 add unit tests
 public final class URLUtils
@@ -79,9 +80,9 @@ else if (urlA.equals(urlB))
    * in EPUB (to test for remote resources compared to container URLs).
    * 
    * @param test
-   *          the URL to test
+   *        the URL to test
    * @param local
-   *          the URL it is tested against
+   *        the URL it is tested against
    * @return `true` if and only if `test` is remote compared to `local`.
    */
   public static boolean isRemote(URL test, URL local)
@@ -151,13 +152,33 @@ public static String decode(String string)
     return percentDecode(string);
   }
 
+  public static URL normalize(URL url)
+  {
+    URL normalized = url;
+    if (url != null)
+    {
+      try
+      {
+        if (url.isHierarchical() && url.path() != null)
+        {
+          normalized = url.withPath(URLUtils.encodePath(URLUtils.decode(url.path())));
+        }
+        normalized = new DecodeUnreservedCanonicalizer().canonicalize(normalized);
+      } catch (GalimatiasParseException unexpected)
+      {
+        throw new AssertionError(unexpected);
+      }
+    }
+    return normalized;
+  }
+
   /**
    * Returns the MIME type of a `data:` URL.
    * 
    * @param url
-   *          a URL, can be `null`.
+   *        a URL, can be `null`.
    * @return the MIME type declared in the data URL (can be an empty string), or
-   *         `null` if `url` is not a data URL.
+   *           `null` if `url` is not a data URL.
    */
   public static String getDataURLType(URL url)
   {
diff --git a/src/test/resources/epub3/04-ocf/files/url-percent-encoded-valid/EPUB/content&001.xhtml b/src/test/resources/epub3/04-ocf/files/url-percent-encoded-valid/EPUB/content&001.xhtml
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html xmlns:epub="https://www.idpf.org/2007/ops" xmlns="https://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+	<head>
+		<meta charset="utf-8"/>
+		<title>Minimal EPUB</title>
+	</head>
+	<body>
+		<h1>Loomings</h1>
+		<p>Call me Ishmael.</p>
+	</body>
+</html>
diff --git a/src/test/resources/epub3/04-ocf/files/url-percent-encoded-valid/EPUB/nav.xhtml b/src/test/resources/epub3/04-ocf/files/url-percent-encoded-valid/EPUB/nav.xhtml
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html xmlns="https://www.w3.org/1999/xhtml" xmlns:epub="https://www.idpf.org/2007/ops" xml:lang="en" lang="en">
+	<head>
+		<meta charset="utf-8"/>
+		<title>Minimal Nav</title>
+	</head>
+	<body>
+		<nav epub:type="toc">
+			<ol>
+				<li><a href="content%26001.xhtml">content 001</a></li>
+			</ol>
+		</nav>
+	</body>
+</html>
diff --git a/src/test/resources/epub3/04-ocf/files/url-percent-encoded-valid/EPUB/package.opf b/src/test/resources/epub3/04-ocf/files/url-percent-encoded-valid/EPUB/package.opf
@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<package xmlns="https://www.idpf.org/2007/opf" version="3.0" xml:lang="en" unique-identifier="q">
+<metadata xmlns:dc="https://purl.org/dc/elements/1.1/">
+  <dc:title id="title">Minimal EPUB 3.0</dc:title>
+  <dc:language>en</dc:language>
+  <dc:identifier id="q">NOID</dc:identifier>
+  <meta property="dcterms:modified">2017-06-14T00:00:01Z</meta>
+</metadata>
+<manifest>
+  <item id="content_001"  href="content%26001.xhtml" media-type="application/xhtml+xml"/>
+  <item id="nav"  href="nav.xhtml" media-type="application/xhtml+xml" properties="nav"/>
+</manifest>
+<spine>
+  <itemref idref="content_001" />
+</spine>
+</package>
diff --git a/src/test/resources/epub3/04-ocf/files/url-percent-encoded-valid/META-INF/container.xml b/src/test/resources/epub3/04-ocf/files/url-percent-encoded-valid/META-INF/container.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<container version="1.0" xmlns="urn:oasis:names:tc:opendocument:xmlns:container">
+	<rootfiles>
+		<rootfile full-path="EPUB/package.opf" media-type="application/oebps-package+xml"/>
+	</rootfiles>
+</container>
diff --git a/src/test/resources/epub3/04-ocf/files/url-percent-encoded-valid/mimetype b/src/test/resources/epub3/04-ocf/files/url-percent-encoded-valid/mimetype
@@ -0,0 +1 @@
+application/epub+zip
diff --git a/src/test/resources/epub3/04-ocf/ocf.feature b/src/test/resources/epub3/04-ocf/ocf.feature
@@ -114,6 +114,12 @@ Feature: EPUB 3 — Open Container Format
     When checking EPUB 'url-in-xhtml-valid.xhtml'
     And no errors or warnings are reported
 
+  @spec @xref:sec-container-iri
+  Scenario: Allow percent-encoded URLs
+    When checking EPUB 'url-percent-encoded-valid'
+    And no errors or warnings are reported
+
+
   #### Invalid container URLs
 
   @spec @xref:sec-container-iri

Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,14 @@ public OCFContainer(Builder builder)`
`79`	`79`
`80`	`80`	`public boolean contains(URL resource)`
`81`	`81`	`{`
`82`		`- return resources.containsKey(resource);`
	`82`	`+ if (resources.containsKey(resource))`
	`83`	`+ {`
	`84`	`+ return true;`
	`85`	`+ }`
	`86`	`+ else`
	`87`	`+ {`
	`88`	`+ return resources.containsKey(URLUtils.normalize(resource));`
	`89`	`+ }`
`83`	`90`	`}`
`84`	`91`
`85`	`92`	`@Override`
`@@ -134,5 +141,4 @@ public boolean isRemote(URL url)`
`134`	`141`	`}`
`135`	`142`	`}`
`136`	`143`
`137`		`-`
`138`	`144`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,8 @@`
`2`	`2`
`3`	`3`	`import java.net.URI;`
`4`	`4`
	`5`	`+import org.w3c.epubcheck.util.url.URLUtils;`
	`6`	`+`
`5`	`7`	`import com.adobe.epubcheck.api.EPUBLocation;`
`6`	`8`	`import com.adobe.epubcheck.api.Report;`
`7`	`9`	`import com.adobe.epubcheck.messages.MessageId;`
`@@ -64,7 +66,7 @@ public URL setBase(String newBase, EPUBLocation location)`
`64`	`66`
`65`	`67`	`public URL checkURL(String string, EPUBLocation location)`
`66`	`68`	`{`
`67`		`- URL url = resolveURL(string, false, location);`
	`69`	`+ URL url = URLUtils.normalize(resolveURL(string, false, location));`
`68`	`70`	`return url;`
`69`	`71`	`}`
`70`	`72`