From 8177dafe39b373a6740cc857bafd9d369121f088 Mon Sep 17 00:00:00 2001
From: Yuri Schimke <yuri@schimke.ee>
Date: Wed, 30 Dec 2020 10:10:09 +0000
Subject: [PATCH 1/6] Fail earlier on cache corruptions

---
 okhttp/src/main/kotlin/okhttp3/Cache.kt       | 49 +++++++++++--------
 okhttp/src/main/kotlin/okhttp3/HttpUrl.kt     | 19 ++++---
 .../test/java/okhttp3/CacheCorruptionTest.kt  | 34 ++++++++-----
 3 files changed, 60 insertions(+), 42 deletions(-)

diff --git a/okhttp/src/main/kotlin/okhttp3/Cache.kt b/okhttp/src/main/kotlin/okhttp3/Cache.kt
index 82baf682d63d..b73d1e6a7d4d 100644
--- a/okhttp/src/main/kotlin/okhttp3/Cache.kt
+++ b/okhttp/src/main/kotlin/okhttp3/Cache.kt
@@ -15,16 +15,7 @@
  */
 package okhttp3
 
-import java.io.Closeable
-import java.io.File
-import java.io.Flushable
-import java.io.IOException
-import java.security.cert.Certificate
-import java.security.cert.CertificateEncodingException
-import java.security.cert.CertificateException
-import java.security.cert.CertificateFactory
-import java.util.NoSuchElementException
-import java.util.TreeSet
+import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
 import okhttp3.MediaType.Companion.toMediaTypeOrNull
 import okhttp3.internal.EMPTY_HEADERS
 import okhttp3.internal.cache.CacheRequest
@@ -48,6 +39,23 @@ import okio.ForwardingSource
 import okio.Sink
 import okio.Source
 import okio.buffer
+import java.io.Closeable
+import java.io.File
+import java.io.Flushable
+import java.io.IOException
+import java.security.cert.Certificate
+import java.security.cert.CertificateEncodingException
+import java.security.cert.CertificateException
+import java.security.cert.CertificateFactory
+import java.util.*
+import kotlin.collections.ArrayList
+import kotlin.collections.List
+import kotlin.collections.MutableIterator
+import kotlin.collections.MutableSet
+import kotlin.collections.Set
+import kotlin.collections.emptyList
+import kotlin.collections.emptySet
+import kotlin.collections.none
 
 /**
  * Caches HTTP and HTTPS responses to the filesystem so they may be reused, saving time and
@@ -425,7 +433,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 +444,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 +498,10 @@ 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()
+        url = urlLine.toHttpUrlOrNull() ?: throw IOException("Cache Failure for $urlLine")
         requestMethod = source.readUtf8LineStrict()
         val varyHeadersBuilder = Headers.Builder()
         val varyRequestHeaderLineCount = readInt(source)
@@ -536,13 +545,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 +564,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 +625,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 +636,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
index 84c3bd6691bb..f7b0842f940d 100644
--- a/okhttp/src/main/kotlin/okhttp3/HttpUrl.kt
+++ b/okhttp/src/main/kotlin/okhttp3/HttpUrl.kt
@@ -15,15 +15,6 @@
  */
 package okhttp3
 
-import java.net.InetAddress
-import java.net.MalformedURLException
-import java.net.URI
-import java.net.URISyntaxException
-import java.net.URL
-import java.nio.charset.Charset
-import java.nio.charset.StandardCharsets.UTF_8
-import java.util.Collections
-import java.util.LinkedHashSet
 import okhttp3.HttpUrl.Companion.toHttpUrl
 import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
 import okhttp3.internal.canParseAsIpAddress
@@ -34,6 +25,14 @@ import okhttp3.internal.parseHexDigit
 import okhttp3.internal.publicsuffix.PublicSuffixDatabase
 import okhttp3.internal.toCanonicalHost
 import okio.Buffer
+import java.net.InetAddress
+import java.net.MalformedURLException
+import java.net.URI
+import java.net.URISyntaxException
+import java.net.URL
+import java.nio.charset.Charset
+import java.nio.charset.StandardCharsets.UTF_8
+import java.util.*
 
 /**
  * A uniform resource locator (URL) with a scheme of either `http` or `https`. Use this class to
@@ -1258,7 +1257,7 @@ class HttpUrl internal constructor(
         this.scheme = base.scheme
       } else {
         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 $input")
       }
 
       // Authority.
diff --git a/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt b/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt
index 934fdfb7db53..a10c9149be01 100644
--- a/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt
+++ b/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt
@@ -15,17 +15,6 @@
  */
 package okhttp3
 
-import java.io.File
-import java.net.CookieManager
-import java.net.ResponseCache
-import java.text.DateFormat
-import java.text.SimpleDateFormat
-import java.util.Date
-import java.util.Locale
-import java.util.TimeZone
-import java.util.concurrent.TimeUnit
-import javax.net.ssl.HostnameVerifier
-import javax.net.ssl.SSLSession
 import mockwebserver3.MockResponse
 import mockwebserver3.MockWebServer
 import okhttp3.internal.buildCache
@@ -37,6 +26,15 @@ import org.junit.jupiter.api.AfterEach
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.extension.RegisterExtension
+import java.io.File
+import java.net.CookieManager
+import java.net.ResponseCache
+import java.text.DateFormat
+import java.text.SimpleDateFormat
+import java.util.*
+import java.util.concurrent.TimeUnit
+import javax.net.ssl.HostnameVerifier
+import javax.net.ssl.SSLSession
 
 class CacheCorruptionTest(
   var server: MockWebServer
@@ -100,6 +98,20 @@ class CacheCorruptionTest(
     assertThat(cache.hitCount()).isEqualTo(0)
   }
 
+  @Test fun corruptedUrl() {
+    val response = testCorruptingCache {
+      corruptMetadata {
+        // string http scheme
+        it.substring(5)
+      }
+    }
+
+    assertThat(response.body!!.string()).isEqualTo("ABC.2") // not cached
+    assertThat(cache.requestCount()).isEqualTo(2)
+    assertThat(cache.networkCount()).isEqualTo(2)
+    assertThat(cache.hitCount()).isEqualTo(0)
+  }
+
   private fun corruptMetadata(corruptor: (String) -> String) {
     val metadataFile = fileSystem.files.keys.find { it.name.endsWith(".0") }
     val metadataBuffer = fileSystem.files[metadataFile]

From dea3db42a3e6463fb7ab1b13fc614ed88bd386cc Mon Sep 17 00:00:00 2001
From: Yuri Schimke <yuri@schimke.ee>
Date: Wed, 30 Dec 2020 10:19:30 +0000
Subject: [PATCH 2/6] log warning

---
 okhttp/src/main/kotlin/okhttp3/Cache.kt             | 7 ++++++-
 okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/okhttp/src/main/kotlin/okhttp3/Cache.kt b/okhttp/src/main/kotlin/okhttp3/Cache.kt
index b73d1e6a7d4d..f93882c7fb67 100644
--- a/okhttp/src/main/kotlin/okhttp3/Cache.kt
+++ b/okhttp/src/main/kotlin/okhttp3/Cache.kt
@@ -27,6 +27,7 @@ import okhttp3.internal.http.HttpMethod
 import okhttp3.internal.http.StatusLine
 import okhttp3.internal.io.FileSystem
 import okhttp3.internal.platform.Platform
+import okhttp3.internal.platform.Platform.Companion.WARN
 import okhttp3.internal.toLongOrDefault
 import okio.Buffer
 import okio.BufferedSink
@@ -501,7 +502,11 @@ class Cache internal constructor(
       rawSource.use {
         val source = rawSource.buffer()
         val urlLine = source.readUtf8LineStrict()
-        url = urlLine.toHttpUrlOrNull() ?: throw IOException("Cache Failure for $urlLine")
+        // Choice here is between failing with a correct RuntimeException
+        // or mostly silently with an IOException
+        url = urlLine.toHttpUrlOrNull() ?: throw IOException("Cache Failure for $urlLine").also {
+          Platform.get().log("cache corruption", WARN, it)
+        }
         requestMethod = source.readUtf8LineStrict()
         val varyHeadersBuilder = Headers.Builder()
         val varyRequestHeaderLineCount = readInt(source)
diff --git a/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt b/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt
index a10c9149be01..e4c8e89ab17b 100644
--- a/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt
+++ b/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt
@@ -47,7 +47,7 @@ class CacheCorruptionTest(
   private lateinit var client: OkHttpClient
   private lateinit var cache: Cache
   private val NULL_HOSTNAME_VERIFIER =
-    HostnameVerifier { name: String?, session: SSLSession? -> true }
+    HostnameVerifier { _: String?, _: SSLSession? -> true }
   private val cookieManager = CookieManager()
 
   @BeforeEach fun setUp() {

From 4a85eef8116a33db2608991e42e0f4608841fed3 Mon Sep 17 00:00:00 2001
From: Yuri Schimke <yuri@schimke.ee>
Date: Wed, 30 Dec 2020 10:22:27 +0000
Subject: [PATCH 3/6] fix imports

---
 okhttp/src/main/kotlin/okhttp3/Cache.kt             | 3 ++-
 okhttp/src/main/kotlin/okhttp3/HttpUrl.kt           | 3 ++-
 okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt | 4 +++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/okhttp/src/main/kotlin/okhttp3/Cache.kt b/okhttp/src/main/kotlin/okhttp3/Cache.kt
index f93882c7fb67..fc9549e250f7 100644
--- a/okhttp/src/main/kotlin/okhttp3/Cache.kt
+++ b/okhttp/src/main/kotlin/okhttp3/Cache.kt
@@ -48,7 +48,8 @@ import java.security.cert.Certificate
 import java.security.cert.CertificateEncodingException
 import java.security.cert.CertificateException
 import java.security.cert.CertificateFactory
-import java.util.*
+import java.util.NoSuchElementException
+import java.util.TreeSet
 import kotlin.collections.ArrayList
 import kotlin.collections.List
 import kotlin.collections.MutableIterator
diff --git a/okhttp/src/main/kotlin/okhttp3/HttpUrl.kt b/okhttp/src/main/kotlin/okhttp3/HttpUrl.kt
index f7b0842f940d..3158b9bab217 100644
--- a/okhttp/src/main/kotlin/okhttp3/HttpUrl.kt
+++ b/okhttp/src/main/kotlin/okhttp3/HttpUrl.kt
@@ -32,7 +32,8 @@ import java.net.URISyntaxException
 import java.net.URL
 import java.nio.charset.Charset
 import java.nio.charset.StandardCharsets.UTF_8
-import java.util.*
+import java.util.Collections
+import java.util.LinkedHashSet
 
 /**
  * A uniform resource locator (URL) with a scheme of either `http` or `https`. Use this class to
diff --git a/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt b/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt
index e4c8e89ab17b..14b0975203db 100644
--- a/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt
+++ b/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt
@@ -31,7 +31,9 @@ import java.net.CookieManager
 import java.net.ResponseCache
 import java.text.DateFormat
 import java.text.SimpleDateFormat
-import java.util.*
+import java.util.Date
+import java.util.Locale
+import java.util.TimeZone
 import java.util.concurrent.TimeUnit
 import javax.net.ssl.HostnameVerifier
 import javax.net.ssl.SSLSession

From 41c67d8f920a8afdbba69075e20a632305a939a0 Mon Sep 17 00:00:00 2001
From: Yuri Schimke <yuri@schimke.ee>
Date: Wed, 30 Dec 2020 10:23:35 +0000
Subject: [PATCH 4/6] Fix comment

---
 okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt b/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt
index 14b0975203db..801245037e15 100644
--- a/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt
+++ b/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt
@@ -103,7 +103,7 @@ class CacheCorruptionTest(
   @Test fun corruptedUrl() {
     val response = testCorruptingCache {
       corruptMetadata {
-        // string http scheme
+        // strip https scheme
         it.substring(5)
       }
     }

From 9b70d1913d5954205b96e980b78f038832227680 Mon Sep 17 00:00:00 2001
From: Yuri Schimke <yuri@schimke.ee>
Date: Wed, 30 Dec 2020 10:35:23 +0000
Subject: [PATCH 5/6] Fix tests

---
 okhttp/src/test/java/okhttp3/HttpUrlTest.java | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/okhttp/src/test/java/okhttp3/HttpUrlTest.java b/okhttp/src/test/java/okhttp3/HttpUrlTest.java
index da072f76b7ee..0dc17cf1b4cc 100644
--- a/okhttp/src/test/java/okhttp3/HttpUrlTest.java
+++ b/okhttp/src/test/java/okhttp3/HttpUrlTest.java
@@ -137,7 +137,7 @@ public void parseDoesNotTrimOtherWhitespaceCharacters() throws Exception {
 
     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://host/");
     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'");
@@ -147,11 +147,11 @@ public void parseDoesNotTrimOtherWhitespaceCharacters() throws Exception {
 
   @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");
+    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 #fragment");
   }
 
   @Test

From 4b8efb647ac57953e07a8d460749561431403cb9 Mon Sep 17 00:00:00 2001
From: Yuri Schimke <yuri@schimke.ee>
Date: Sat, 9 Jan 2021 12:47:00 +0000
Subject: [PATCH 6/6] Review comments

---
 okhttp/src/main/kotlin/okhttp3/Cache.kt       | 2 +-
 okhttp/src/main/kotlin/okhttp3/HttpUrl.kt     | 3 ++-
 okhttp/src/test/java/okhttp3/HttpUrlTest.java | 4 ++--
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/okhttp/src/main/kotlin/okhttp3/Cache.kt b/okhttp/src/main/kotlin/okhttp3/Cache.kt
index fc9549e250f7..f505a1488343 100644
--- a/okhttp/src/main/kotlin/okhttp3/Cache.kt
+++ b/okhttp/src/main/kotlin/okhttp3/Cache.kt
@@ -505,7 +505,7 @@ class Cache internal constructor(
         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 Failure for $urlLine").also {
+        url = urlLine.toHttpUrlOrNull() ?: throw IOException("Cache corruption for $urlLine").also {
           Platform.get().log("cache corruption", WARN, it)
         }
         requestMethod = source.readUtf8LineStrict()
diff --git a/okhttp/src/main/kotlin/okhttp3/HttpUrl.kt b/okhttp/src/main/kotlin/okhttp3/HttpUrl.kt
index 3158b9bab217..fe9f76344a80 100644
--- 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 scheme was found for $input")
+            "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
index 0dc17cf1b4cc..59ae43c7538d 100644
--- a/okhttp/src/test/java/okhttp3/HttpUrlTest.java
+++ b/okhttp/src/test/java/okhttp3/HttpUrlTest.java
@@ -137,7 +137,7 @@ public void parseDoesNotTrimOtherWhitespaceCharacters() throws Exception {
 
     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 scheme was found for 0ttp://host/");
+    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'");
@@ -151,7 +151,7 @@ public void parseNoScheme() throws Exception {
     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 #fragment");
+    assertInvalid("#fragment", "Expected URL scheme 'http' or 'https' but no scheme was found for #fragm...");
   }
 
   @Test