RUM-15591: Expose request/response payloads on HttpRequestInfo/HttpResponseInfo
#3714
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| /* | ||
| * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. | ||
| * This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| * Copyright 2016-Present Datadog, Inc. | ||
| */ | ||
| package com.datadog.android.api.instrumentation.network | ||
|
|
||
| import java.nio.charset.Charset | ||
| import java.util.Locale | ||
|
|
||
| /** | ||
| * An immutable, size-capped copy of an HTTP request or response payload. | ||
| * | ||
| * Instances are produced by [HttpRequestInfo.peekBody] and [HttpResponseInfo.peekBody]. Taking a | ||
| * snapshot never consumes the underlying payload: the request or response stays fully readable by | ||
| * the application. | ||
| * | ||
| * @property bytes the captured payload, holding at most the number of bytes requested from | ||
| * [HttpRequestInfo.peekBody] / [HttpResponseInfo.peekBody]. This array is not defensively copied, | ||
| * so it must be treated as read-only. | ||
| * @property contentType the MIME type declared for the payload, including any `charset` parameter, | ||
| * or null when the payload declared none. | ||
| * @property isTruncated true when the payload was larger than the requested maximum and [bytes] | ||
| * therefore only holds its beginning. | ||
| */ | ||
| class HttpBodySnapshot( | ||
| val bytes: ByteArray, | ||
| val contentType: String? = null, | ||
| val isTruncated: Boolean = false | ||
| ) { | ||
|
|
||
| /** | ||
| * Decodes [bytes] into a [String], using the charset declared in [contentType] and falling back | ||
| * to UTF-8 when it declares none or declares one this device doesn't support. | ||
| * | ||
| * Note that when [isTruncated] is true the payload was cut at a byte boundary, so the last | ||
| * character of the result may be garbled for multi-byte encodings. | ||
| * | ||
| * @return the decoded payload. | ||
| */ | ||
| fun string(): String = String(bytes, resolveCharset(contentType)) | ||
|
|
||
| /** @inheritDoc */ | ||
| override fun equals(other: Any?): Boolean { | ||
| if (this === other) return true | ||
| if (other !is HttpBodySnapshot) return false | ||
|
|
||
| return bytes.contentEquals(other.bytes) && | ||
| contentType == other.contentType && | ||
| isTruncated == other.isTruncated | ||
| } | ||
|
|
||
| /** @inheritDoc */ | ||
| override fun hashCode(): Int { | ||
| var result = bytes.contentHashCode() | ||
| result = HASH_MULTIPLIER * result + (contentType?.hashCode() ?: 0) | ||
| result = HASH_MULTIPLIER * result + isTruncated.hashCode() | ||
| return result | ||
| } | ||
|
|
||
| // The payload itself is deliberately left out: it may be large and may hold sensitive data. | ||
| /** @inheritDoc */ | ||
| override fun toString(): String = | ||
| "HttpBodySnapshot(size=${bytes.size}, contentType=$contentType, isTruncated=$isTruncated)" | ||
|
|
||
| companion object { | ||
|
|
||
| /** | ||
| * The number of bytes [HttpRequestInfo.peekBody] and [HttpResponseInfo.peekBody] capture | ||
| * when no explicit maximum is given: 512 KB. | ||
| * | ||
| * A snapshot is held in memory on top of the payload the networking library already holds, | ||
| * so this cap keeps the duplication bounded for large downloads and uploads. | ||
| */ | ||
| const val DEFAULT_MAX_BODY_BYTES: Long = 512L * 1024L | ||
|
|
||
| private const val HASH_MULTIPLIER = 31 | ||
| private const val CHARSET_PARAMETER = "charset" | ||
|
|
||
| private fun resolveCharset(contentType: String?): Charset { | ||
|
Contributor
There was a problem hiding this comment. I am bit afraid this method might be silently complex from the memory perspective because of transformations. For instance - firstOrNull { it.lowercase(Locale.US) } - will create a so in case if this method would be called multiple times in a short period - it could create a pressure to the GC. I am not sure that you could completley avoid that, but manual iteration over the string, might be cheaper. Good test coverage would be required as well for all supported HTTP header format
Contributor
There was a problem hiding this comment.
Then you can defer the lowercasing cost only when a match is found "$CHARSET_PARAMETER=" should probably also be a const so you don't pay the cost of building the search string sequence iteration |
||
| val charsetName = contentType | ||
| ?.split(';') | ||
| ?.asSequence() | ||
| ?.map { it.trim() } | ||
| ?.firstOrNull { it.lowercase(Locale.US).startsWith("$CHARSET_PARAMETER=") } | ||
| ?.substringAfter('=') | ||
| ?.trim('"', ' ') | ||
| ?: return Charsets.UTF_8 | ||
|
|
||
| return try { | ||
| @Suppress("UnsafeThirdPartyFunctionCall") // IllegalArgumentException is caught | ||
| Charset.forName(charsetName) | ||
| } catch (@Suppress("SwallowedException") _: IllegalArgumentException) { | ||
| // covers both IllegalCharsetNameException and UnsupportedCharsetException | ||
| Charsets.UTF_8 | ||
| } | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,166 @@ | ||
| /* | ||
| * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. | ||
| * This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| * Copyright 2016-Present Datadog, Inc. | ||
| */ | ||
|
|
||
| package com.datadog.android.api.instrumentation.network | ||
|
|
||
| import com.datadog.android.utils.forge.Configurator | ||
| import fr.xgouchet.elmyr.annotation.StringForgery | ||
| import fr.xgouchet.elmyr.junit5.ForgeConfiguration | ||
| import fr.xgouchet.elmyr.junit5.ForgeExtension | ||
| import org.assertj.core.api.Assertions.assertThat | ||
| import org.junit.jupiter.api.Test | ||
| import org.junit.jupiter.api.extension.ExtendWith | ||
| import org.junit.jupiter.api.extension.Extensions | ||
| import org.junit.jupiter.params.ParameterizedTest | ||
| import org.junit.jupiter.params.provider.ValueSource | ||
| import org.mockito.junit.jupiter.MockitoSettings | ||
| import org.mockito.quality.Strictness | ||
|
|
||
| @Extensions( | ||
| ExtendWith(ForgeExtension::class) | ||
| ) | ||
| @MockitoSettings(strictness = Strictness.LENIENT) | ||
| @ForgeConfiguration(Configurator::class) | ||
| internal class HttpBodySnapshotTest { | ||
|
|
||
| @Test | ||
| fun `M decode as UTF-8 W string() {no content type}`(@StringForgery fakePayload: String) { | ||
| // Given | ||
| val testedSnapshot = HttpBodySnapshot(fakePayload.toByteArray(Charsets.UTF_8)) | ||
|
|
||
| // When | ||
| val result = testedSnapshot.string() | ||
|
|
||
| // Then | ||
| assertThat(result).isEqualTo(fakePayload) | ||
| } | ||
|
|
||
| @Test | ||
| fun `M decode as UTF-8 W string() {content type without charset}`(@StringForgery fakePayload: String) { | ||
| // Given | ||
| val testedSnapshot = HttpBodySnapshot( | ||
| bytes = fakePayload.toByteArray(Charsets.UTF_8), | ||
| contentType = "application/json" | ||
| ) | ||
|
|
||
| // When | ||
| val result = testedSnapshot.string() | ||
|
|
||
| // Then | ||
| assertThat(result).isEqualTo(fakePayload) | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @ValueSource( | ||
| strings = [ | ||
| "text/plain; charset=ISO-8859-1", | ||
| "text/plain;charset=ISO-8859-1", | ||
| "text/plain; charset=iso-8859-1", | ||
| "text/plain; CHARSET=ISO-8859-1", | ||
| "text/plain; charset=\"ISO-8859-1\"", | ||
| "text/plain; boundary=xyz; charset=ISO-8859-1" | ||
| ] | ||
| ) | ||
| fun `M decode with declared charset W string() {charset parameter}`(fakeContentType: String) { | ||
| // Given | ||
| val fakePayload = "café naïve" | ||
| val testedSnapshot = HttpBodySnapshot( | ||
| bytes = fakePayload.toByteArray(Charsets.ISO_8859_1), | ||
| contentType = fakeContentType | ||
| ) | ||
|
|
||
| // When | ||
| val result = testedSnapshot.string() | ||
|
|
||
| // Then | ||
| assertThat(result).isEqualTo(fakePayload) | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @ValueSource( | ||
| strings = [ | ||
| "text/plain; charset=not-a-real-charset", | ||
| "text/plain; charset=<>!", | ||
| "text/plain; charset=" | ||
| ] | ||
| ) | ||
| fun `M fall back to UTF-8 W string() {unusable charset}`(fakeContentType: String) { | ||
| // Given | ||
| val fakePayload = "café naïve" | ||
| val testedSnapshot = HttpBodySnapshot( | ||
| bytes = fakePayload.toByteArray(Charsets.UTF_8), | ||
| contentType = fakeContentType | ||
| ) | ||
|
|
||
| // When | ||
| val result = testedSnapshot.string() | ||
|
|
||
| // Then | ||
| assertThat(result).isEqualTo(fakePayload) | ||
| } | ||
|
|
||
| @Test | ||
| fun `M not be truncated by default W constructor()`(@StringForgery fakePayload: String) { | ||
| // When | ||
| val testedSnapshot = HttpBodySnapshot(fakePayload.toByteArray()) | ||
|
|
||
| // Then | ||
| assertThat(testedSnapshot.isTruncated).isFalse() | ||
| assertThat(testedSnapshot.contentType).isNull() | ||
| } | ||
|
|
||
| @Test | ||
| fun `M compare payload content W equals() {same content}`( | ||
| @StringForgery fakePayload: String, | ||
| @StringForgery fakeContentType: String | ||
| ) { | ||
| // Given | ||
| val testedSnapshot = HttpBodySnapshot(fakePayload.toByteArray(), fakeContentType, true) | ||
| val other = HttpBodySnapshot(fakePayload.toByteArray(), fakeContentType, true) | ||
|
|
||
| // Then | ||
| assertThat(testedSnapshot).isEqualTo(other) | ||
| assertThat(testedSnapshot.hashCode()).isEqualTo(other.hashCode()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `M differ W equals() {different payload}`( | ||
| @StringForgery fakePayload: String, | ||
| @StringForgery fakeOtherPayload: String | ||
| ) { | ||
| // Given | ||
| val testedSnapshot = HttpBodySnapshot(fakePayload.toByteArray()) | ||
| val other = HttpBodySnapshot((fakePayload + fakeOtherPayload).toByteArray()) | ||
|
|
||
| // Then | ||
| assertThat(testedSnapshot).isNotEqualTo(other) | ||
| } | ||
|
|
||
| @Test | ||
| fun `M differ W equals() {different truncation flag}`(@StringForgery fakePayload: String) { | ||
| // Given | ||
| val testedSnapshot = HttpBodySnapshot(fakePayload.toByteArray(), isTruncated = true) | ||
| val other = HttpBodySnapshot(fakePayload.toByteArray(), isTruncated = false) | ||
|
|
||
| // Then | ||
| assertThat(testedSnapshot).isNotEqualTo(other) | ||
| } | ||
|
|
||
| @Test | ||
| fun `M not leak the payload W toString()`(@StringForgery(size = 32) fakePayload: String) { | ||
| // Given | ||
| val testedSnapshot = HttpBodySnapshot(fakePayload.toByteArray(), "application/json", true) | ||
|
|
||
| // When | ||
| val result = testedSnapshot.toString() | ||
|
|
||
| // Then | ||
| assertThat(result).doesNotContain(fakePayload) | ||
| assertThat(result).contains("size=${fakePayload.toByteArray().size}") | ||
| assertThat(result).contains("contentType=application/json") | ||
| assertThat(result).contains("isTruncated=true") | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it's immutable - maybe better to declare as
data class