diff --git a/okhttp/src/main/kotlin/okhttp3/Cache.kt b/okhttp/src/main/kotlin/okhttp3/Cache.kt index 82baf682d63d..f505a1488343 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 @@ -36,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 @@ -48,6 +40,24 @@ 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.NoSuchElementException +import java.util.TreeSet +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 +435,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 +446,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 +500,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", WARN, it) + } requestMethod = source.readUtf8LineStrict() val varyHeadersBuilder = Headers.Builder() val varyRequestHeaderLineCount = readInt(source) @@ -536,13 +551,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 +570,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 +631,8 @@ class Cache internal constructor( private fun writeCertList(sink: BufferedSink, certificates: List) { 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 +642,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..fe9f76344a80 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,15 @@ 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.Collections +import java.util.LinkedHashSet /** * A uniform resource locator (URL) with a scheme of either `http` or `https`. Use this class to @@ -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/CacheCorruptionTest.kt b/okhttp/src/test/java/okhttp3/CacheCorruptionTest.kt index 934fdfb7db53..801245037e15 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,17 @@ 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.Date +import java.util.Locale +import java.util.TimeZone +import java.util.concurrent.TimeUnit +import javax.net.ssl.HostnameVerifier +import javax.net.ssl.SSLSession class CacheCorruptionTest( var server: MockWebServer @@ -49,7 +49,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() { @@ -100,6 +100,20 @@ class CacheCorruptionTest( assertThat(cache.hitCount()).isEqualTo(0) } + @Test fun corruptedUrl() { + val response = testCorruptingCache { + corruptMetadata { + // strip https 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] diff --git a/okhttp/src/test/java/okhttp3/HttpUrlTest.java b/okhttp/src/test/java/okhttp3/HttpUrlTest.java index da072f76b7ee..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 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'"); @@ -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 #fragm..."); } @Test