From 5dbe0eeba10d4972a8b920ee00691197cb385d70 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Thu, 27 Apr 2023 15:06:53 +0000 Subject: [PATCH] [Table logging] Restrict strings to a max length. Fixes: 271854154 Test: atest TableLogBufferTest TableChangeTest Change-Id: I4d1f5e6bc1385e6b27a412a4703e27bd0338cc05 --- .../android/systemui/log/table/TableChange.kt | 33 ++-- .../systemui/log/table/TableLogBuffer.kt | 2 +- .../systemui/log/table/TableChangeTest.kt | 150 ++++++++++++++++++ .../systemui/log/table/TableLogBufferTest.kt | 107 +++++++++++++ 4 files changed, 281 insertions(+), 11 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/log/table/TableChange.kt b/packages/SystemUI/src/com/android/systemui/log/table/TableChange.kt index 42fdd689df6ce..592044e514bec 100644 --- a/packages/SystemUI/src/com/android/systemui/log/table/TableChange.kt +++ b/packages/SystemUI/src/com/android/systemui/log/table/TableChange.kt @@ -27,19 +27,27 @@ import androidx.annotation.VisibleForTesting */ data class TableChange( var timestamp: Long = 0, - var columnPrefix: String = "", - var columnName: String = "", - var isInitial: Boolean = false, - var type: DataType = DataType.EMPTY, - var bool: Boolean = false, - var int: Int? = null, - var str: String? = null, + private var columnPrefix: String = "", + private var columnName: String = "", + private var isInitial: Boolean = false, + private var type: DataType = DataType.EMPTY, + private var bool: Boolean = false, + private var int: Int? = null, + private var str: String? = null, ) { + init { + // Truncate any strings that were passed into the constructor. [reset] and [set] will take + // care of the rest of the truncation. + this.columnPrefix = columnPrefix.take(MAX_STRING_LENGTH) + this.columnName = columnName.take(MAX_STRING_LENGTH) + this.str = str?.take(MAX_STRING_LENGTH) + } + /** Resets to default values so that the object can be recycled. */ fun reset(timestamp: Long, columnPrefix: String, columnName: String, isInitial: Boolean) { this.timestamp = timestamp - this.columnPrefix = columnPrefix - this.columnName = columnName + this.columnPrefix = columnPrefix.take(MAX_STRING_LENGTH) + this.columnName = columnName.take(MAX_STRING_LENGTH) this.isInitial = isInitial this.type = DataType.EMPTY this.bool = false @@ -50,7 +58,7 @@ data class TableChange( /** Sets this to store a string change. */ fun set(value: String?) { type = DataType.STRING - str = value + str = value?.take(MAX_STRING_LENGTH) } /** Sets this to store a boolean change. */ @@ -89,6 +97,8 @@ data class TableChange( } } + fun getColumnName() = columnName + fun getVal(): String { val value = when (type) { @@ -109,5 +119,8 @@ data class TableChange( companion object { @VisibleForTesting const val IS_INITIAL_PREFIX = "**" + // Don't allow any strings larger than this length so that we have a hard upper limit on the + // size of the data stored by the buffer. + @VisibleForTesting const val MAX_STRING_LENGTH = 500 } } diff --git a/packages/SystemUI/src/com/android/systemui/log/table/TableLogBuffer.kt b/packages/SystemUI/src/com/android/systemui/log/table/TableLogBuffer.kt index 9d883cc10d6c8..8babfc90d2690 100644 --- a/packages/SystemUI/src/com/android/systemui/log/table/TableLogBuffer.kt +++ b/packages/SystemUI/src/com/android/systemui/log/table/TableLogBuffer.kt @@ -291,7 +291,7 @@ class TableLogBuffer( private fun echoToDesiredEndpoints(change: TableChange) { if ( logcatEchoTracker.isBufferLoggable(bufferName = name, LogLevel.DEBUG) || - logcatEchoTracker.isTagLoggable(change.columnName, LogLevel.DEBUG) + logcatEchoTracker.isTagLoggable(change.getColumnName(), LogLevel.DEBUG) ) { if (change.hasData()) { localLogcat.d(name, change.logcatRepresentation()) diff --git a/packages/SystemUI/tests/src/com/android/systemui/log/table/TableChangeTest.kt b/packages/SystemUI/tests/src/com/android/systemui/log/table/TableChangeTest.kt index a003e1d9d1f3f..43e430fd7fa24 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/log/table/TableChangeTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/log/table/TableChangeTest.kt @@ -19,7 +19,9 @@ package com.android.systemui.log.table import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.log.table.TableChange.Companion.IS_INITIAL_PREFIX +import com.android.systemui.log.table.TableChange.Companion.MAX_STRING_LENGTH import com.google.common.truth.Truth.assertThat +import junit.framework.Assert.assertTrue import org.junit.Test @SmallTest @@ -326,4 +328,152 @@ class TableChangeTest : SysuiTestCase() { assertThat(underTest.getVal()).doesNotContain(IS_INITIAL_PREFIX) } + + @Test + fun constructor_columnAndValueTooLong_truncated() { + val underTest = + TableChange( + columnPrefix = "P".repeat(MAX_STRING_LENGTH + 10), + columnName = "N".repeat(MAX_STRING_LENGTH + 10), + type = TableChange.DataType.STRING, + str = "V".repeat(MAX_STRING_LENGTH + 10), + ) + + assertThat(underTest.getName()).contains("P".repeat(MAX_STRING_LENGTH)) + assertThat(underTest.getName()).doesNotContain("P".repeat(MAX_STRING_LENGTH + 1)) + assertThat(underTest.getName()).contains("N".repeat(MAX_STRING_LENGTH)) + assertThat(underTest.getName()).doesNotContain("N".repeat(MAX_STRING_LENGTH + 1)) + assertThat(underTest.getVal()).isEqualTo("V".repeat(MAX_STRING_LENGTH)) + } + + @Test + fun constructor_columnNameNotTooLong_noReallocation() { + val inputColumnName = "fakeName" + val inputValue = "fakeValue" + val underTest = + TableChange( + columnPrefix = "", + columnName = inputColumnName, + type = TableChange.DataType.STRING, + str = inputValue, + ) + + // Use referential equality to verify we didn't reallocate a new string when the string is + // *not* too long. + assertTrue(underTest.getColumnName() === inputColumnName) + } + + @Test + fun reset_columnPrefixTooLong_truncated() { + val underTest = TableChange() + + underTest.reset( + timestamp = 1L, + columnPrefix = "P".repeat(MAX_STRING_LENGTH + 10), + columnName = "name", + isInitial = false, + ) + + assertThat(underTest.getName()).contains("P".repeat(MAX_STRING_LENGTH)) + assertThat(underTest.getName()).doesNotContain("P".repeat(MAX_STRING_LENGTH + 1)) + } + + @Test + fun reset_columnNameTooLong_truncated() { + val underTest = TableChange() + + underTest.reset( + timestamp = 1L, + columnPrefix = "prefix", + columnName = "N".repeat(MAX_STRING_LENGTH + 10), + isInitial = false, + ) + + assertThat(underTest.getName()).contains("N".repeat(MAX_STRING_LENGTH)) + assertThat(underTest.getName()).doesNotContain("N".repeat(MAX_STRING_LENGTH + 1)) + } + + @Test + fun reset_columnNameNotTooLong_noReallocation() { + val underTest = TableChange() + val shortColumnName = "shortColumnName" + + underTest.reset( + timestamp = 1L, + columnPrefix = "prefix", + columnName = shortColumnName, + isInitial = false, + ) + + // Use referential equality to verify we didn't reallocate a new string when the string is + // *not* too long. + assertTrue(underTest.getColumnName() === shortColumnName) + } + + @Test + fun setString_valueTooLong_truncated() { + val underTest = TableChange() + + underTest.set("V".repeat(MAX_STRING_LENGTH + 1)) + + assertThat(underTest.getVal()).isEqualTo("V".repeat(MAX_STRING_LENGTH)) + } + + @Test + fun updateTo_newColumnPrefixTooLong_truncated() { + val underTest = TableChange(columnPrefix = "fakePrefix", columnName = "fakeName") + underTest.set(42) + + val new = + TableChange( + columnPrefix = "P".repeat(MAX_STRING_LENGTH + 10), + columnName = "name", + ) + underTest.updateTo(new) + + assertThat(underTest.getName()).contains("P".repeat(MAX_STRING_LENGTH)) + assertThat(underTest.getName()).doesNotContain("P".repeat(MAX_STRING_LENGTH + 1)) + } + + @Test + fun updateTo_newColumnNameTooLong_truncated() { + val underTest = TableChange(columnPrefix = "fakePrefix", columnName = "fakeName") + underTest.set(42) + + val new = + TableChange( + columnPrefix = "prefix", + columnName = "N".repeat(MAX_STRING_LENGTH + 10), + ) + underTest.updateTo(new) + + assertThat(underTest.getName()).contains("N".repeat(MAX_STRING_LENGTH)) + assertThat(underTest.getName()).doesNotContain("N".repeat(MAX_STRING_LENGTH + 1)) + } + + @Test + fun updateTo_columnNameNotTooLong_noReallocation() { + val underTest = TableChange() + val shortColumnName = "shortColumnName" + val new = TableChange(columnPrefix = "prefix", columnName = shortColumnName) + + underTest.updateTo(new) + + // Use referential equality to verify we didn't reallocate a new string when the string is + // *not* too long. + assertTrue(underTest.getColumnName() === shortColumnName) + } + + @Test + fun updateTo_newValTooLong_truncated() { + val underTest = TableChange(columnPrefix = "fakePrefix", columnName = "fakeName") + underTest.set("value") + + val new = TableChange() + new.set("V".repeat(MAX_STRING_LENGTH + 10)) + + underTest.updateTo(new) + + assertThat(underTest.getVal()).isEqualTo("V".repeat(MAX_STRING_LENGTH)) + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/log/table/TableLogBufferTest.kt b/packages/SystemUI/tests/src/com/android/systemui/log/table/TableLogBufferTest.kt index a2b2322899b77..f867fc7bf84ae 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/log/table/TableLogBufferTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/log/table/TableLogBufferTest.kt @@ -19,6 +19,7 @@ package com.android.systemui.log.table import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.log.table.TableChange.Companion.IS_INITIAL_PREFIX +import com.android.systemui.log.table.TableChange.Companion.MAX_STRING_LENGTH import com.android.systemui.plugins.log.LogLevel import com.android.systemui.plugins.log.LogcatEchoTracker import com.android.systemui.util.mockito.any @@ -575,6 +576,112 @@ class TableLogBufferTest : SysuiTestCase() { assertThat(dumpedString).contains(expected6) } + @Test + fun dumpChanges_tooLongColumnPrefix_viaLogChange_truncated() { + underTest.logChange( + prefix = "P".repeat(MAX_STRING_LENGTH + 10), + columnName = "name", + value = true, + ) + + val dumpedString = dumpChanges() + + assertThat(dumpedString).contains("P".repeat(MAX_STRING_LENGTH)) + assertThat(dumpedString).doesNotContain("P".repeat(MAX_STRING_LENGTH + 1)) + } + + @Test + fun dumpChanges_tooLongColumnPrefix_viaLogDiffs_truncated() { + val prevDiffable = object : TestDiffable() {} + val nextDiffable = + object : TestDiffable() { + override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) { + row.logChange("status", "value") + } + } + + // WHEN the column prefix is too large + underTest.logDiffs( + columnPrefix = "P".repeat(MAX_STRING_LENGTH + 10), + prevDiffable, + nextDiffable, + ) + + val dumpedString = dumpChanges() + + // THEN it's truncated to the max length + assertThat(dumpedString).contains("P".repeat(MAX_STRING_LENGTH)) + assertThat(dumpedString).doesNotContain("P".repeat(MAX_STRING_LENGTH + 1)) + } + + @Test + fun dumpChanges_tooLongColumnName_viaLogChange_truncated() { + underTest.logChange( + prefix = "prefix", + columnName = "N".repeat(MAX_STRING_LENGTH + 10), + value = 10, + ) + + val dumpedString = dumpChanges() + + assertThat(dumpedString).contains("N".repeat(MAX_STRING_LENGTH)) + assertThat(dumpedString).doesNotContain("N".repeat(MAX_STRING_LENGTH + 1)) + } + + @Test + fun dumpChanges_tooLongColumnName_viaLogDiffs_truncated() { + val prevDiffable = object : TestDiffable() {} + val nextDiffable = + object : TestDiffable() { + override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) { + // WHEN the column name is too large + row.logChange(columnName = "N".repeat(MAX_STRING_LENGTH + 10), "value") + } + } + + underTest.logDiffs(columnPrefix = "prefix", prevDiffable, nextDiffable) + + val dumpedString = dumpChanges() + + // THEN it's truncated to the max length + assertThat(dumpedString).contains("N".repeat(MAX_STRING_LENGTH)) + assertThat(dumpedString).doesNotContain("N".repeat(MAX_STRING_LENGTH + 1)) + } + + @Test + fun dumpChanges_tooLongValue_viaLogChange_truncated() { + underTest.logChange( + prefix = "prefix", + columnName = "name", + value = "V".repeat(MAX_STRING_LENGTH + 10), + ) + + val dumpedString = dumpChanges() + + assertThat(dumpedString).contains("V".repeat(MAX_STRING_LENGTH)) + assertThat(dumpedString).doesNotContain("V".repeat(MAX_STRING_LENGTH + 1)) + } + + @Test + fun dumpChanges_tooLongValue_viaLogDiffs_truncated() { + val prevDiffable = object : TestDiffable() {} + val nextDiffable = + object : TestDiffable() { + override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) { + // WHEN the value is too large + row.logChange("columnName", value = "V".repeat(MAX_STRING_LENGTH + 10)) + } + } + + underTest.logDiffs(columnPrefix = "prefix", prevDiffable, nextDiffable) + + val dumpedString = dumpChanges() + + // THEN it's truncated to the max length + assertThat(dumpedString).contains("V".repeat(MAX_STRING_LENGTH)) + assertThat(dumpedString).doesNotContain("V".repeat(MAX_STRING_LENGTH + 1)) + } + @Test fun dumpChanges_rotatesIfBufferIsFull() { lateinit var valToDump: String