Backport of #6495 (#6940)

square · Nov 29, 2021 · acf8735 · acf8735
1 parent 86acfbd
commit acf8735
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 19 deletions.
diff --git a/okhttp/src/main/kotlin/okhttp3/Cache.kt b/okhttp/src/main/kotlin/okhttp3/Cache.kt
@@ -15,6 +15,7 @@
  */
 package okhttp3
 
+import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
 import java.io.Closeable
 import java.io.File
 import java.io.Flushable
@@ -425,7 +426,7 @@ class Cache internal constructor(
   }
 
   private class Entry {
-    private val url: String
+    private val url: HttpUrl
     private val varyHeaders: Headers
     private val requestMethod: String
     private val protocol: Protocol
@@ -436,7 +437,7 @@ class Cache internal constructor(
     private val sentRequestMillis: Long
     private val receivedResponseMillis: Long
 
-    private val isHttps: Boolean get() = url.startsWith("https://")
+    private val isHttps: Boolean get() = url.scheme == "https"
 
     /**
      * Reads an entry from an input stream. A typical entry looks like this:
@@ -490,9 +491,14 @@ class Cache internal constructor(
      * optional. If present, it contains the TLS version.
      */
     @Throws(IOException::class) constructor(rawSource: Source) {
-      try {
+      rawSource.use {
         val source = rawSource.buffer()
-        url = source.readUtf8LineStrict()
+        val urlLine = source.readUtf8LineStrict()
+        // Choice here is between failing with a correct RuntimeException
+        // or mostly silently with an IOException
+        url = urlLine.toHttpUrlOrNull() ?: throw IOException("Cache corruption for $urlLine").also {
+          Platform.get().log("cache corruption", Platform.WARN, it)
+        }
         requestMethod = source.readUtf8LineStrict()
         val varyHeadersBuilder = Headers.Builder()
         val varyRequestHeaderLineCount = readInt(source)
@@ -536,13 +542,11 @@ class Cache internal constructor(
         } else {
           handshake = null
         }
-      } finally {
-        rawSource.close()
       }
     }
 
     constructor(response: Response) {
-      this.url = response.request.url.toString()
+      this.url = response.request.url
       this.varyHeaders = response.varyHeaders()
       this.requestMethod = response.request.method
       this.protocol = response.protocol
@@ -557,7 +561,7 @@ class Cache internal constructor(
     @Throws(IOException::class)
     fun writeTo(editor: DiskLruCache.Editor) {
       editor.newSink(ENTRY_METADATA).buffer().use { sink ->
-        sink.writeUtf8(url).writeByte('\n'.toInt())
+        sink.writeUtf8(url.toString()).writeByte('\n'.toInt())
         sink.writeUtf8(requestMethod).writeByte('\n'.toInt())
         sink.writeDecimalLong(varyHeaders.size.toLong()).writeByte('\n'.toInt())
         for (i in 0 until varyHeaders.size) {
@@ -618,8 +622,8 @@ class Cache internal constructor(
     private fun writeCertList(sink: BufferedSink, certificates: List<Certificate>) {
       try {
         sink.writeDecimalLong(certificates.size.toLong()).writeByte('\n'.toInt())
-        for (i in 0 until certificates.size) {
-          val bytes = certificates[i].encoded
+        for (element in certificates) {
+          val bytes = element.encoded
           val line = bytes.toByteString().base64()
           sink.writeUtf8(line).writeByte('\n'.toInt())
         }
@@ -629,7 +633,7 @@ class Cache internal constructor(
     }
 
     fun matches(request: Request, response: Response): Boolean {
-      return url == request.url.toString() &&
+      return url == request.url &&
           requestMethod == request.method &&
           varyMatches(response, varyHeaders, request)
     }

diff --git a/okhttp/src/main/kotlin/okhttp3/HttpUrl.kt b/okhttp/src/main/kotlin/okhttp3/HttpUrl.kt
@@ -1257,8 +1257,9 @@ class HttpUrl internal constructor(
       } else if (base != null) {
         this.scheme = base.scheme
       } else {
+        val truncated = if (input.length > 6) input.take(6) + "..." else input
         throw IllegalArgumentException(
-            "Expected URL scheme 'http' or 'https' but no colon was found")
+            "Expected URL scheme 'http' or 'https' but no scheme was found for $truncated")
       }
 
       // Authority.

diff --git a/okhttp/src/test/java/okhttp3/HttpUrlTest.java b/okhttp/src/test/java/okhttp3/HttpUrlTest.java
@@ -144,20 +144,21 @@ HttpUrl parse(String url) {
 
     assertInvalid("image640://480.png", "Expected URL scheme 'http' or 'https' but was 'image640'");
     assertInvalid("httpp://host/", "Expected URL scheme 'http' or 'https' but was 'httpp'");
-    assertInvalid("0ttp://host/", "Expected URL scheme 'http' or 'https' but no colon was found");
+    assertInvalid("0ttp://host/", "Expected URL scheme 'http' or 'https' but no scheme was found for 0ttp:/...");
     assertInvalid("ht+tp://host/", "Expected URL scheme 'http' or 'https' but was 'ht+tp'");
     assertInvalid("ht.tp://host/", "Expected URL scheme 'http' or 'https' but was 'ht.tp'");
     assertInvalid("ht-tp://host/", "Expected URL scheme 'http' or 'https' but was 'ht-tp'");
     assertInvalid("ht1tp://host/", "Expected URL scheme 'http' or 'https' but was 'ht1tp'");
     assertInvalid("httpss://host/", "Expected URL scheme 'http' or 'https' but was 'httpss'");
   }
 
-  @Test public void parseNoScheme() throws Exception {
-    assertInvalid("//host", "Expected URL scheme 'http' or 'https' but no colon was found");
-    assertInvalid("/path", "Expected URL scheme 'http' or 'https' but no colon was found");
-    assertInvalid("path", "Expected URL scheme 'http' or 'https' but no colon was found");
-    assertInvalid("?query", "Expected URL scheme 'http' or 'https' but no colon was found");
-    assertInvalid("#fragment", "Expected URL scheme 'http' or 'https' but no colon was found");
+  @Test
+  public void parseNoScheme() throws Exception {
+    assertInvalid("//host", "Expected URL scheme 'http' or 'https' but no scheme was found for //host");
+    assertInvalid("/path", "Expected URL scheme 'http' or 'https' but no scheme was found for /path");
+    assertInvalid("path", "Expected URL scheme 'http' or 'https' but no scheme was found for path");
+    assertInvalid("?query", "Expected URL scheme 'http' or 'https' but no scheme was found for ?query");
+    assertInvalid("#fragment", "Expected URL scheme 'http' or 'https' but no scheme was found for #fragm...");
   }
 
   @Test public void newBuilderResolve() throws Exception {