StrixOS/build
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add fingerprint to packages.map.

Browse Source 
No guards to this change because we will guard actually writing the
fingerprint, and right now new storage is not in trunkfood yet. This
change modifies the package map file structure. Note that if the new
storage was in trunkfood, this could (theoretically) cause issues if
there were cross-container READ_WRITE flags (not permitted per
documentation) and if the containers were built at separate aconfig
versions (ie before and after this change). Adding the fingerprint will
help prevent such issues in the future. Incremented the storage version
number as I've changed the format.

Again, fingerprint is not actually written in this CL, it always has a
value of 0.

Updated the test files as well to have the new version and the
fingerprint. Since this changed the package node size, some of the
information in the buckets there (offset) has changed as well.

Also added a test util for flags from another package to test future
changes.

Bug: 316357686
Test: atest aconfig.test
Change-Id: I09e10808492f241fe78028d2757f7d63328623c3
This commit is contained in:
Marybeth Fair
2024-09-05 11:38:50 -04:00
parent 112c4886c7
commit 769d8eed37
30 changed files with 201 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
tools/aconfig/aconfig/Android.bp
View File

@@ -68,6 +68,14 @@ aconfig_values {
],
}
aconfig_values {
name: "aconfig.test.flag.second_values",
package: "com.android.aconfig.test",
srcs: [
"tests/third.values",
],
}
aconfig_value_set {
name: "aconfig.test.flag.value_set",
values: [

74
tools/aconfig/aconfig/src/commands.rs
View File

@@ -17,7 +17,7 @@
use anyhow::{bail, ensure, Context, Result};
use itertools::Itertools;
use protobuf::Message;
use std::collections::{BTreeMap, HashMap};
use std::collections::HashMap;
use std::hash::Hasher;
use std::io::Read;
use std::path::PathBuf;
@@ -422,25 +422,32 @@ where
Ok(flag_ids)
}
#[allow(dead_code)] // TODO: b/316357686 - Use fingerprint in codegen to
// protect hardcoded offset reads.
pub fn compute_flag_offsets_fingerprint(flags_map: &HashMap<String, u16>) -> Result<u64> {
// Creates a fingerprint of the flag names. Sorts the vector.
pub fn compute_flags_fingerprint(flag_names: &mut Vec<String>) -> Result<u64> {
flag_names.sort();
let mut hasher = SipHasher13::new();
// Need to sort to ensure the data is added to the hasher in the same order
// each run.
let sorted_map: BTreeMap<&String, &u16> = flags_map.iter().collect();
for (flag, offset) in sorted_map {
// See https://docs.rs/siphasher/latest/siphasher/#note for use of write
// over write_i16. Similarly, use to_be_bytes rather than to_ne_bytes to
// ensure consistency.
for flag in flag_names {
hasher.write(flag.as_bytes());
hasher.write(&offset.to_be_bytes());
}
Ok(hasher.finish())
}
#[allow(dead_code)] // TODO: b/316357686 - Use fingerprint in codegen to
// protect hardcoded offset reads.
fn compute_fingerprint_from_parsed_flags(flags: ProtoParsedFlags) -> Result<u64> {
let separated_flags: Vec<ProtoParsedFlag> = flags.parsed_flag.into_iter().collect::<Vec<_>>();
// All flags must belong to the same package as the fingerprint is per-package.
let Some(_package) = find_unique_package(&separated_flags) else {
bail!("No parsed flags, or the parsed flags use different packages.");
};
let mut flag_names =
separated_flags.into_iter().map(|flag| flag.name.unwrap()).collect::<Vec<_>>();
compute_flags_fingerprint(&mut flag_names)
}
#[cfg(test)]
mod tests {
use super::*;
@@ -449,15 +456,46 @@ mod tests {
#[test]
fn test_offset_fingerprint() {
let parsed_flags = crate::test::parse_test_flags();
let package = find_unique_package(&parsed_flags.parsed_flag).unwrap().to_string();
let flag_ids = assign_flag_ids(&package, parsed_flags.parsed_flag.iter()).unwrap();
let expected_fingerprint = 10709892481002252132u64;
let expected_fingerprint: u64 = 5801144784618221668;
let hash_result = compute_flag_offsets_fingerprint(&flag_ids);
let hash_result = compute_fingerprint_from_parsed_flags(parsed_flags);
assert_eq!(hash_result.unwrap(), expected_fingerprint);
}
#[test]
fn test_offset_fingerprint_matches_from_package() {
let parsed_flags: ProtoParsedFlags = crate::test::parse_test_flags();
// All test flags are in the same package, so fingerprint from all of them.
let result_from_parsed_flags = compute_fingerprint_from_parsed_flags(parsed_flags.clone());
let mut flag_names_vec = parsed_flags
.parsed_flag
.clone()
.into_iter()
.map(|flag| flag.name.unwrap())
.map(String::from)
.collect::<Vec<_>>();
let result_from_names = compute_flags_fingerprint(&mut flag_names_vec);
// Assert the same hash is generated for each case.
assert_eq!(result_from_parsed_flags.unwrap(), result_from_names.unwrap());
}
#[test]
fn test_offset_fingerprint_different_packages_does_not_match() {
// Parse flags from two packages.
let parsed_flags: ProtoParsedFlags = crate::test::parse_test_flags();
let second_parsed_flags = crate::test::parse_second_package_flags();
let result_from_parsed_flags = compute_fingerprint_from_parsed_flags(parsed_flags).unwrap();
let second_result = compute_fingerprint_from_parsed_flags(second_parsed_flags).unwrap();
// Different flags should have a different fingerprint.
assert_ne!(result_from_parsed_flags, second_result);
}
#[test]
fn test_parse_flags() {
let parsed_flags = crate::test::parse_test_flags(); // calls parse_flags

9
tools/aconfig/aconfig/src/storage/mod.rs
View File

@@ -25,12 +25,14 @@ use crate::storage::{
flag_table::create_flag_table, flag_value::create_flag_value,
package_table::create_package_table,
};
use aconfig_protos::{ProtoParsedFlag, ProtoParsedFlags};
use aconfig_protos::ProtoParsedFlag;
use aconfig_protos::ProtoParsedFlags;
use aconfig_storage_file::StorageFileType;
pub struct FlagPackage<'a> {
pub package_name: &'a str,
pub package_id: u32,
pub fingerprint: u64,
pub flag_names: HashSet<&'a str>,
pub boolean_flags: Vec<&'a ProtoParsedFlag>,
// The index of the first boolean flag in this aconfig package among all boolean
@@ -43,6 +45,7 @@ impl<'a> FlagPackage<'a> {
FlagPackage {
package_name,
package_id,
fingerprint: 0,
flag_names: HashSet::new(),
boolean_flags: vec![],
boolean_start_index: 0,
@@ -78,6 +81,8 @@ where
for p in packages.iter_mut() {
p.boolean_start_index = boolean_start_index;
boolean_start_index += p.boolean_flags.len() as u32;
// TODO: b/316357686 - Calculate fingerprint and add to package.
}
packages
@@ -115,6 +120,8 @@ mod tests {
use super::*;
use crate::Input;
use aconfig_protos::ProtoParsedFlags;
pub fn parse_all_test_flags() -> Vec<ProtoParsedFlags> {
let aconfig_files = [
(

1
tools/aconfig/aconfig/src/storage/package_table.rs
View File

@@ -48,6 +48,7 @@ impl PackageTableNodeWrapper {
let node = PackageTableNode {
package_name: String::from(package.package_name),
package_id: package.package_id,
fingerprint: package.fingerprint,
boolean_start_index: package.boolean_start_index,
next_offset: None,
};

18
tools/aconfig/aconfig/src/test.rs
View File

@@ -295,6 +295,24 @@ parsed_flag {
aconfig_protos::parsed_flags::try_from_binary_proto(&bytes).unwrap()
}
pub fn parse_second_package_flags() -> ProtoParsedFlags {
let bytes = crate::commands::parse_flags(
"com.android.aconfig.second_test",
Some("system"),
vec![Input {
source: "tests/test_second_package.aconfig".to_string(),
reader: Box::new(include_bytes!("../tests/test_second_package.aconfig").as_slice()),
}],
vec![Input {
source: "tests/third.values".to_string(),
reader: Box::new(include_bytes!("../tests/third.values").as_slice()),
}],
crate::commands::DEFAULT_FLAG_PERMISSION,
)
.unwrap();
aconfig_protos::parsed_flags::try_from_binary_proto(&bytes).unwrap()
}
pub fn first_significant_code_diff(a: &str, b: &str) -> Option<String> {
let a = a.lines().map(|line| line.trim_start()).filter(|line| !line.is_empty());
let b = b.lines().map(|line| line.trim_start()).filter(|line| !line.is_empty());

12
tools/aconfig/aconfig/tests/test_second_package.aconfig Normal file
View File

@@ -0,0 +1,12 @@
package: "com.android.aconfig.second_test"
container: "system"
flag {
name: "testing_flag"
namespace: "another_namespace"
description: "This is a flag for testing."
bug: "123"
metadata {
purpose: PURPOSE_UNSPECIFIED
}
}

6
tools/aconfig/aconfig/tests/third.values Normal file
View File

@@ -0,0 +1,6 @@
flag_value {
package: "com.android.aconfig.second_test"
name: "testing_flag"
state: DISABLED
permission: READ_WRITE
}

2
tools/aconfig/aconfig_storage_file/src/flag_info.rs
View File

@@ -227,7 +227,7 @@ mod tests {
let bytes = &flag_info_list.into_bytes();
let mut head = 0;
let version = read_u32_from_bytes(bytes, &mut head).unwrap();
assert_eq!(version, 1);
assert_eq!(version, 2);
}
#[test]

2
tools/aconfig/aconfig_storage_file/src/flag_table.rs
View File

@@ -253,7 +253,7 @@ mod tests {
let bytes = &flag_table.into_bytes();
let mut head = 0;
let version = read_u32_from_bytes(bytes, &mut head).unwrap();
assert_eq!(version, 1);
assert_eq!(version, 2);
}
#[test]

2
tools/aconfig/aconfig_storage_file/src/flag_value.rs
View File

@@ -159,7 +159,7 @@ mod tests {
let bytes = &flag_value_list.into_bytes();
let mut head = 0;
let version = read_u32_from_bytes(bytes, &mut head).unwrap();
assert_eq!(version, 1);
assert_eq!(version, 2);
}
#[test]

12
tools/aconfig/aconfig_storage_file/src/lib.rs
View File

@@ -58,7 +58,7 @@ use crate::AconfigStorageError::{
};
/// Storage file version
pub const FILE_VERSION: u32 = 1;
pub const FILE_VERSION: u32 = 2;
/// Good hash table prime number
pub(crate) const HASH_PRIMES: [u32; 29] = [
@@ -254,6 +254,16 @@ pub fn read_u32_from_bytes(buf: &[u8], head: &mut usize) -> Result<u32, AconfigS
Ok(val)
}
// Read and parse bytes as u64
pub fn read_u64_from_bytes(buf: &[u8], head: &mut usize) -> Result<u64, AconfigStorageError> {
let val =
u64::from_le_bytes(buf[*head..*head + 8].try_into().map_err(|errmsg| {
BytesParseFail(anyhow!("fail to parse u64 from bytes: {}", errmsg))
})?);
*head += 8;
Ok(val)
}
/// Read and parse bytes as string
pub(crate) fn read_str_from_bytes(
buf: &[u8],

18
tools/aconfig/aconfig_storage_file/src/package_table.rs
View File

@@ -17,7 +17,10 @@
//! package table module defines the package table file format and methods for serialization
//! and deserialization
use crate::{get_bucket_index, read_str_from_bytes, read_u32_from_bytes, read_u8_from_bytes};
use crate::{
get_bucket_index, read_str_from_bytes, read_u32_from_bytes, read_u64_from_bytes,
read_u8_from_bytes,
};
use crate::{AconfigStorageError, StorageFileType};
use anyhow::anyhow;
use serde::{Deserialize, Serialize};
@@ -97,6 +100,7 @@ impl PackageTableHeader {
pub struct PackageTableNode {
pub package_name: String,
pub package_id: u32,
pub fingerprint: u64,
// The index of the first boolean flag in this aconfig package among all boolean
// flags in this container.
pub boolean_start_index: u32,
@@ -108,8 +112,12 @@ impl fmt::Debug for PackageTableNode {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
writeln!(
f,
"Package: {}, Id: {}, Boolean flag start index: {}, Next: {:?}",
self.package_name, self.package_id, self.boolean_start_index, self.next_offset
"Package: {}, Id: {}, Fingerprint: {}, Boolean flag start index: {}, Next: {:?}",
self.package_name,
self.package_id,
self.fingerprint,
self.boolean_start_index,
self.next_offset
)?;
Ok(())
}
@@ -123,6 +131,7 @@ impl PackageTableNode {
result.extend_from_slice(&(name_bytes.len() as u32).to_le_bytes());
result.extend_from_slice(name_bytes);
result.extend_from_slice(&self.package_id.to_le_bytes());
result.extend_from_slice(&self.fingerprint.to_le_bytes());
result.extend_from_slice(&self.boolean_start_index.to_le_bytes());
result.extend_from_slice(&self.next_offset.unwrap_or(0).to_le_bytes());
result
@@ -134,6 +143,7 @@ impl PackageTableNode {
let node = Self {
package_name: read_str_from_bytes(bytes, &mut head)?,
package_id: read_u32_from_bytes(bytes, &mut head)?,
fingerprint: read_u64_from_bytes(bytes, &mut head)?,
boolean_start_index: read_u32_from_bytes(bytes, &mut head)?,
next_offset: match read_u32_from_bytes(bytes, &mut head)? {
0 => None,
@@ -251,7 +261,7 @@ mod tests {
let bytes = &package_table.into_bytes();
let mut head = 0;
let version = read_u32_from_bytes(bytes, &mut head).unwrap();
assert_eq!(version, 1);
assert_eq!(version, 2);
}
#[test]

17
tools/aconfig/aconfig_storage_file/src/test_utils.rs
View File

@@ -26,30 +26,33 @@ use tempfile::NamedTempFile;
pub fn create_test_package_table() -> PackageTable {
let header = PackageTableHeader {
version: 1,
version: 2,
container: String::from("mockup"),
file_type: StorageFileType::PackageMap as u8,
file_size: 209,
file_size: 233,
num_packages: 3,
bucket_offset: 31,
node_offset: 59,
};
let buckets: Vec<Option<u32>> = vec![Some(59), None, None, Some(109), None, None, None];
let buckets: Vec<Option<u32>> = vec![Some(59), None, None, Some(117), None, None, None];
let first_node = PackageTableNode {
package_name: String::from("com.android.aconfig.storage.test_2"),
package_id: 1,
fingerprint: 0,
boolean_start_index: 3,
next_offset: None,
};
let second_node = PackageTableNode {
package_name: String::from("com.android.aconfig.storage.test_1"),
package_id: 0,
fingerprint: 0,
boolean_start_index: 0,
next_offset: Some(159),
next_offset: Some(175),
};
let third_node = PackageTableNode {
package_name: String::from("com.android.aconfig.storage.test_4"),
package_id: 2,
fingerprint: 0,
boolean_start_index: 6,
next_offset: None,
};
@@ -78,7 +81,7 @@ impl FlagTableNode {
pub fn create_test_flag_table() -> FlagTable {
let header = FlagTableHeader {
version: 1,
version: 2,
container: String::from("mockup"),
file_type: StorageFileType::FlagMap as u8,
file_size: 321,
@@ -120,7 +123,7 @@ pub fn create_test_flag_table() -> FlagTable {
pub fn create_test_flag_value_list() -> FlagValueList {
let header = FlagValueHeader {
version: 1,
version: 2,
container: String::from("mockup"),
file_type: StorageFileType::FlagVal as u8,
file_size: 35,
@@ -133,7 +136,7 @@ pub fn create_test_flag_value_list() -> FlagValueList {
pub fn create_test_flag_info_list() -> FlagInfoList {
let header = FlagInfoHeader {
version: 1,
version: 2,
container: String::from("mockup"),
file_type: StorageFileType::FlagInfo as u8,
file_size: 35,

4
tools/aconfig/aconfig_storage_file/srcs/android/aconfig/storage/ByteBufferReader.java
View File

@@ -37,6 +37,10 @@ public class ByteBufferReader {
return Short.toUnsignedInt(mByteBuffer.getShort());
}
public long readLong() {
return mByteBuffer.getLong();
}
public int readInt() {
return this.mByteBuffer.getInt();
}

7
tools/aconfig/aconfig_storage_file/srcs/android/aconfig/storage/PackageTable.java
View File

@@ -118,6 +118,7 @@ public class PackageTable {
private String mPackageName;
private int mPackageId;
private long mFingerprint;
private int mBooleanStartIndex;
private int mNextOffset;
@@ -125,6 +126,7 @@ public class PackageTable {
Node node = new Node();
node.mPackageName = reader.readString();
node.mPackageId = reader.readInt();
node.mFingerprint = reader.readLong();
node.mBooleanStartIndex = reader.readInt();
node.mNextOffset = reader.readInt();
node.mNextOffset = node.mNextOffset == 0 ? -1 : node.mNextOffset;
@@ -150,6 +152,7 @@ public class PackageTable {
return Objects.equals(mPackageName, other.mPackageName)
&& mPackageId == other.mPackageId
&& mBooleanStartIndex == other.mBooleanStartIndex
&& mFingerprint == other.mFingerprint
&& mNextOffset == other.mNextOffset;
}
@@ -165,6 +168,10 @@ public class PackageTable {
return mBooleanStartIndex;
}
public long getFingerprint() {
return mFingerprint;
}
public int getNextOffset() {
return mNextOffset;
}

BIN
tools/aconfig/aconfig_storage_file/tests/flag.info
View File

Binary file not shown.

BIN
tools/aconfig/aconfig_storage_file/tests/flag.map
View File

Binary file not shown.

BIN
tools/aconfig/aconfig_storage_file/tests/flag.val
View File

Binary file not shown.

BIN
tools/aconfig/aconfig_storage_file/tests/package.map
View File

Binary file not shown.

2
tools/aconfig/aconfig_storage_file/tests/srcs/FlagTableTest.java
View File

@@ -33,7 +33,7 @@ public class FlagTableTest {
public void testFlagTable_rightHeader() throws Exception {
FlagTable flagTable = FlagTable.fromBytes(TestDataUtils.getTestFlagMapByteBuffer());
FlagTable.Header header = flagTable.getHeader();
assertEquals(1, header.getVersion());
assertEquals(2, header.getVersion());
assertEquals("mockup", header.getContainer());
assertEquals(FileType.FLAG_MAP, header.getFileType());
assertEquals(321, header.getFileSize());

2
tools/aconfig/aconfig_storage_file/tests/srcs/FlagValueListTest.java
View File

@@ -36,7 +36,7 @@ public class FlagValueListTest {
FlagValueList flagValueList =
FlagValueList.fromBytes(TestDataUtils.getTestFlagValByteBuffer());
FlagValueList.Header header = flagValueList.getHeader();
assertEquals(1, header.getVersion());
assertEquals(2, header.getVersion());
assertEquals("mockup", header.getContainer());
assertEquals(FileType.FLAG_VAL, header.getFileType());
assertEquals(35, header.getFileSize());

11
tools/aconfig/aconfig_storage_file/tests/srcs/PackageTableTest.java
View File

@@ -20,7 +20,6 @@ import static org.junit.Assert.assertEquals;
import android.aconfig.storage.FileType;
import android.aconfig.storage.PackageTable;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -30,10 +29,9 @@ public class PackageTableTest {
@Test
public void testPackageTable_rightHeader() throws Exception {
PackageTable packageTable =
PackageTable.fromBytes(TestDataUtils.getTestPackageMapByteBuffer());
PackageTable packageTable = PackageTable.fromBytes(TestDataUtils.getTestPackageMapByteBuffer());
PackageTable.Header header = packageTable.getHeader();
assertEquals(1, header.getVersion());
assertEquals(2, header.getVersion());
assertEquals("mockup", header.getContainer());
assertEquals(FileType.PACKAGE_MAP, header.getFileType());
assertEquals(209, header.getFileSize());
@@ -44,8 +42,7 @@ public class PackageTableTest {
@Test
public void testPackageTable_rightNode() throws Exception {
PackageTable packageTable =
PackageTable.fromBytes(TestDataUtils.getTestPackageMapByteBuffer());
PackageTable packageTable = PackageTable.fromBytes(TestDataUtils.getTestPackageMapByteBuffer());
PackageTable.Node node1 = packageTable.get("com.android.aconfig.storage.test_1");
PackageTable.Node node2 = packageTable.get("com.android.aconfig.storage.test_2");
@@ -63,7 +60,7 @@ public class PackageTableTest {
assertEquals(3, node2.getBooleanStartIndex());
assertEquals(6, node4.getBooleanStartIndex());
assertEquals(159, node1.getNextOffset());
assertEquals(175, node1.getNextOffset());
assertEquals(-1, node2.getNextOffset());
assertEquals(-1, node4.getNextOffset());
}

8
tools/aconfig/aconfig_storage_read_api/src/lib.rs
View File

@@ -507,9 +507,9 @@ mod tests {
#[test]
// this test point locks down flag storage file version number query api
fn test_storage_version_query() {
assert_eq!(get_storage_file_version("./tests/package.map").unwrap(), 1);
assert_eq!(get_storage_file_version("./tests/flag.map").unwrap(), 1);
assert_eq!(get_storage_file_version("./tests/flag.val").unwrap(), 1);
assert_eq!(get_storage_file_version("./tests/flag.info").unwrap(), 1);
assert_eq!(get_storage_file_version("./tests/package.map").unwrap(), 2);
assert_eq!(get_storage_file_version("./tests/flag.map").unwrap(), 2);
assert_eq!(get_storage_file_version("./tests/flag.val").unwrap(), 2);
assert_eq!(get_storage_file_version("./tests/flag.info").unwrap(), 2);
}
}

BIN
tools/aconfig/aconfig_storage_read_api/tests/flag.info
View File

Binary file not shown.

BIN
tools/aconfig/aconfig_storage_read_api/tests/flag.map
View File

Binary file not shown.

BIN
tools/aconfig/aconfig_storage_read_api/tests/flag.val
View File

Binary file not shown.

BIN
tools/aconfig/aconfig_storage_read_api/tests/package.map
View File

Binary file not shown.

8
tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.cpp
View File

@@ -80,16 +80,16 @@ class AconfigStorageTest : public ::testing::Test {
TEST_F(AconfigStorageTest, test_storage_version_query) {
auto version = api::get_storage_file_version(package_map);
ASSERT_TRUE(version.ok());
ASSERT_EQ(*version, 1);
ASSERT_EQ(*version, 2);
version = api::get_storage_file_version(flag_map);
ASSERT_TRUE(version.ok());
ASSERT_EQ(*version, 1);
ASSERT_EQ(*version, 2);
version = api::get_storage_file_version(flag_val);
ASSERT_TRUE(version.ok());
ASSERT_EQ(*version, 1);
ASSERT_EQ(*version, 2);
version = api::get_storage_file_version(flag_info);
ASSERT_TRUE(version.ok());
ASSERT_EQ(*version, 1);
ASSERT_EQ(*version, 2);
}
/// Negative test to lock down the error when mapping none exist storage files

8
tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.rs
View File

@@ -200,9 +200,9 @@ mod aconfig_storage_rust_test {
#[test]
fn test_storage_version_query() {
assert_eq!(get_storage_file_version("./package.map").unwrap(), 1);
assert_eq!(get_storage_file_version("./flag.map").unwrap(), 1);
assert_eq!(get_storage_file_version("./flag.val").unwrap(), 1);
assert_eq!(get_storage_file_version("./flag.info").unwrap(), 1);
assert_eq!(get_storage_file_version("./package.map").unwrap(), 2);
assert_eq!(get_storage_file_version("./flag.map").unwrap(), 2);
assert_eq!(get_storage_file_version("./flag.val").unwrap(), 2);
assert_eq!(get_storage_file_version("./flag.info").unwrap(), 2);
}
}