Skip to content

Commit

Permalink
Handle empty and missing response bodies. (#250)
Browse files Browse the repository at this point in the history
* Add failing test cases.
* Remove unused const.
* Gzip response body if it was gunzipped.
* Add test cases for regular bodies in Chucker.
* Fix rule formatting.
* Use proper name for application interceptor.
* Return original response downstream.
* Account for no content with gzip encoding.
* Use Truth for Chucker tests.
* Honor empty plaintext bodies.
* Revert changes to HttpBinClient.
* Update library/src/test/java/com/chuckerteam/chucker/ChuckerInterceptorDelegate.kt
* Update library/src/main/java/com/chuckerteam/chucker/internal/support/OkHttpUtils.kt
Co-authored-by: Nicola Corti <corti.nico@gmail.com>
Co-authored-by: Volodymyr Buberenko <vbuberen@users.noreply.github.com>
  • Loading branch information
MiSikora authored Mar 30, 2020
1 parent ef1b63e commit 930f009
Show file tree
Hide file tree
Showing 7 changed files with 282 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import com.chuckerteam.chucker.internal.support.IOUtils
import com.chuckerteam.chucker.internal.support.TeeSource
import com.chuckerteam.chucker.internal.support.contentLength
import com.chuckerteam.chucker.internal.support.contentType
import com.chuckerteam.chucker.internal.support.hasBody
import com.chuckerteam.chucker.internal.support.isGzipped
import java.io.File
import java.io.IOException
Expand Down Expand Up @@ -179,7 +180,7 @@ class ChuckerInterceptor internal constructor(
transaction: HttpTransaction
): Response {
val responseBody = response.body()
if (responseBody == null) {
if (!response.hasBody() || responseBody == null) {
collector.onResponseReceived(transaction)
return response
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,37 @@
package com.chuckerteam.chucker.internal.support

import java.net.HttpURLConnection.HTTP_NOT_MODIFIED
import java.net.HttpURLConnection.HTTP_NO_CONTENT
import java.net.HttpURLConnection.HTTP_OK
import okhttp3.Headers
import okhttp3.Request
import okhttp3.Response

private const val HTTP_CONTINUE = 100

/** Returns true if the response must have a (possibly 0-length) body. See RFC 7231. */
internal fun Response.hasBody(): Boolean {
// HEAD requests never yield a body regardless of the response headers.
if (request().method() == "HEAD") {
return false
}

val responseCode = code()
if ((responseCode < HTTP_CONTINUE || responseCode >= HTTP_OK) &&
(responseCode != HTTP_NO_CONTENT) &&
(responseCode != HTTP_NOT_MODIFIED)
) {
return true
}

// If the Content-Length or Transfer-Encoding headers disagree with the response code, the
// response is malformed. For best compatibility, we honor the headers.
return ((contentLength > 0) || isChunked)
}

internal val Response.contentLength: Long
get() {
return this.header("Content-Length")?.toLongOrNull() ?: -1
return this.header("Content-Length")?.toLongOrNull() ?: -1L
}

internal val Response.isChunked: Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ internal class TeeSource(
sideStream.close()
upstream.close()
if (isUpstreamExhausted) {
callback.onSuccess(sideChannel)
// Failure might have occurred due to exceeding limit.
if (!isFailure) {
callback.onSuccess(sideChannel)
}
} else {
callSideChannelFailure(IOException("Upstream was not fully consumed"))
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package com.chuckerteam.chucker

import android.content.Context
import com.chuckerteam.chucker.api.ChuckerCollector
import com.chuckerteam.chucker.api.ChuckerInterceptor
import com.chuckerteam.chucker.internal.data.entity.HttpTransaction
import com.chuckerteam.chucker.internal.support.FileFactory
import io.mockk.every
import io.mockk.mockk
import java.util.concurrent.CopyOnWriteArrayList
import java.util.concurrent.atomic.AtomicLong
import okhttp3.Interceptor
import okhttp3.Response

internal class ChuckerInterceptorDelegate(
fileFactory: FileFactory,
maxContentLength: Long = 250000L,
headersToRedact: Set<String> = emptySet()
) : Interceptor {
private val idGenerator = AtomicLong()
private val transactions = CopyOnWriteArrayList<HttpTransaction>()

private val mockContext = mockk<Context> {
every { getString(any()) } returns ""
}
private val mockCollector = mockk<ChuckerCollector> {
every { onRequestSent(any()) } returns Unit
every { onResponseReceived(any()) } answers {
val transaction = (args[0] as HttpTransaction)
transaction.id = idGenerator.getAndIncrement()
transactions.add(transaction)
}
}

private val chucker = ChuckerInterceptor(
context = mockContext,
collector = mockCollector,
maxContentLength = maxContentLength,
headersToRedact = headersToRedact,
fileFactory = fileFactory
)

internal fun expectTransaction(): HttpTransaction {
if (transactions.isEmpty()) {
throw AssertionError("Expected transaction but was empty.")
}
return transactions.removeAt(0)
}

internal fun expectNoTransactions() {
if (transactions.isNotEmpty()) {
throw AssertionError("Expected no transactions but found ${transactions.size}")
}
}

override fun intercept(chain: Interceptor.Chain): Response {
return chucker.intercept(chain)
}
}
11 changes: 11 additions & 0 deletions library/src/test/java/com/chuckerteam/chucker/TestUtils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ package com.chuckerteam.chucker

import androidx.lifecycle.LiveData
import androidx.lifecycle.Observer
import com.chuckerteam.chucker.internal.support.hasBody
import java.io.File
import okhttp3.Response
import okio.Buffer
import okio.ByteString
import okio.Okio

fun getResourceFile(file: String): Buffer {
Expand All @@ -12,6 +15,14 @@ fun getResourceFile(file: String): Buffer {
}
}

fun Response.readByteStringBody(): ByteString? {
return if (hasBody()) {
body()?.source()?.use { it.readByteString() }
} else {
null
}
}

fun <T> LiveData<T>.test(test: LiveDataRecord<T>.() -> Unit) {
val observer = RecordingObserver<T>()
observeForever(observer)
Expand Down
Loading

0 comments on commit 930f009

Please sign in to comment.