Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit cfc5f5e9 authored by Treehugger Robot's avatar Treehugger Robot Committed by Gerrit Code Review
Browse files

Merge "aconfig: restrict valid namespace and flag names"

parents d5d51e21 00cf045c
Loading
Loading
Loading
Loading
+10 −9
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ use serde::{Deserialize, Serialize};
use std::io::{Read, Write};

use crate::aconfig::{FlagDeclaration, FlagState, FlagValue, Permission};
use crate::codegen;
use crate::commands::Source;

const DEFAULT_FLAG_STATE: FlagState = FlagState::Disabled;
@@ -108,7 +109,7 @@ pub struct CacheBuilder {

impl CacheBuilder {
    pub fn new(namespace: String) -> Result<CacheBuilder> {
        ensure!(!namespace.is_empty(), "empty namespace");
        ensure!(codegen::is_valid_identifier(&namespace), "bad namespace");
        let cache = Cache { namespace, items: vec![] };
        Ok(CacheBuilder { cache })
    }
@@ -118,7 +119,7 @@ impl CacheBuilder {
        source: Source,
        declaration: FlagDeclaration,
    ) -> Result<&mut CacheBuilder> {
        ensure!(!declaration.name.is_empty(), "empty flag name");
        ensure!(codegen::is_valid_identifier(&declaration.name), "bad flag name");
        ensure!(!declaration.description.is_empty(), "empty flag description");
        ensure!(
            self.cache.items.iter().all(|item| item.name != declaration.name),
@@ -146,8 +147,8 @@ impl CacheBuilder {
        source: Source,
        value: FlagValue,
    ) -> Result<&mut CacheBuilder> {
        ensure!(!value.namespace.is_empty(), "empty flag namespace");
        ensure!(!value.name.is_empty(), "empty flag name");
        ensure!(codegen::is_valid_identifier(&value.namespace), "bad flag namespace");
        ensure!(codegen::is_valid_identifier(&value.name), "bad flag name");
        ensure!(
            value.namespace == self.cache.namespace,
            "failed to set values for flag {}/{} from {}: expected namespace {}",
@@ -270,14 +271,14 @@ mod tests {
            .add_flag_value(
                Source::Memory,
                FlagValue {
                    namespace: "some-other-namespace".to_string(),
                    namespace: "some_other_namespace".to_string(),
                    name: "foo".to_string(),
                    state: FlagState::Enabled,
                    permission: Permission::ReadOnly,
                },
            )
            .unwrap_err();
        assert_eq!(&format!("{:?}", error), "failed to set values for flag some-other-namespace/foo from <memory>: expected namespace ns");
        assert_eq!(&format!("{:?}", error), "failed to set values for flag some_other_namespace/foo from <memory>: expected namespace ns");

        let cache = builder.build();
        let item = cache.iter().find(|&item| item.name == "foo").unwrap();
@@ -300,7 +301,7 @@ mod tests {
                FlagDeclaration { name: "".to_string(), description: "Description".to_string() },
            )
            .unwrap_err();
        assert_eq!(&format!("{:?}", error), "empty flag name");
        assert_eq!(&format!("{:?}", error), "bad flag name");

        let error = builder
            .add_flag_declaration(
@@ -332,7 +333,7 @@ mod tests {
                },
            )
            .unwrap_err();
        assert_eq!(&format!("{:?}", error), "empty flag namespace");
        assert_eq!(&format!("{:?}", error), "bad flag namespace");

        let error = builder
            .add_flag_value(
@@ -345,7 +346,7 @@ mod tests {
                },
            )
            .unwrap_err();
        assert_eq!(&format!("{:?}", error), "empty flag name");
        assert_eq!(&format!("{:?}", error), "bad flag name");
    }

    #[test]
+43 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2023 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

pub fn is_valid_identifier(s: &str) -> bool {
    // Identifiers must match [a-z][a-z0-9_]*
    let mut chars = s.chars();
    let Some(first) = chars.next() else {
        return false;
    };
    if !first.is_ascii_lowercase() {
        return false;
    }
    chars.all(|ch| ch.is_ascii_lowercase() || ch.is_ascii_digit() || ch == '_')
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_is_valid_identifier() {
        assert!(is_valid_identifier("foo"));
        assert!(is_valid_identifier("foo_bar_123"));

        assert!(!is_valid_identifier(""));
        assert!(!is_valid_identifier("123_foo"));
        assert!(!is_valid_identifier("foo-bar"));
        assert!(!is_valid_identifier("foo-b\u{00e5}r"));
    }
}
+5 −5
Original line number Diff line number Diff line
@@ -31,7 +31,7 @@ pub fn generate_java_code(cache: &Cache) -> Result<OutputFile> {
    let mut template = TinyTemplate::new();
    template.add_template("java_code_gen", include_str!("../templates/java.template"))?;
    let contents = template.render("java_code_gen", &context)?;
    let mut path: PathBuf = namespace.split('.').collect();
    let mut path: PathBuf = ["aconfig", namespace].iter().collect();
    // TODO: Allow customization of the java class name
    path.push("Flags.java");
    Ok(OutputFile { contents: contents.into(), path })
@@ -76,7 +76,7 @@ mod tests {

    #[test]
    fn test_generate_java_code() {
        let namespace = "com.example";
        let namespace = "example";
        let mut builder = CacheBuilder::new(namespace.to_string()).unwrap();
        builder
            .add_flag_declaration(
@@ -106,7 +106,7 @@ mod tests {
            )
            .unwrap();
        let cache = builder.build();
        let expect_content = r#"package com.example;
        let expect_content = r#"package aconfig.example;

        import android.provider.DeviceConfig;

@@ -118,7 +118,7 @@ mod tests {

            public static boolean test2() {
                return DeviceConfig.getBoolean(
                    "com.example",
                    "example",
                    "test2__test2",
                    false
                );
@@ -127,7 +127,7 @@ mod tests {
        }
        "#;
        let file = generate_java_code(&cache).unwrap();
        assert_eq!("com/example/Flags.java", file.path.to_str().unwrap());
        assert_eq!("aconfig/example/Flags.java", file.path.to_str().unwrap());
        assert_eq!(
            expect_content.replace(' ', ""),
            String::from_utf8(file.contents).unwrap().replace(' ', "")
+6 −6
Original line number Diff line number Diff line
@@ -23,10 +23,10 @@ use crate::cache::{Cache, Item};
use crate::commands::OutputFile;

pub fn generate_rust_code(cache: &Cache) -> Result<OutputFile> {
    let namespace = cache.namespace().to_lowercase();
    let namespace = cache.namespace();
    let parsed_flags: Vec<TemplateParsedFlag> =
        cache.iter().map(|item| create_template_parsed_flag(&namespace, item)).collect();
    let context = TemplateContext { namespace, parsed_flags };
        cache.iter().map(|item| create_template_parsed_flag(namespace, item)).collect();
    let context = TemplateContext { namespace: namespace.to_string(), parsed_flags };
    let mut template = TinyTemplate::new();
    template.add_template("rust_code_gen", include_str!("../templates/rust.template"))?;
    let contents = template.render("rust_code_gen", &context)?;
@@ -56,7 +56,7 @@ struct TemplateParsedFlag {
fn create_template_parsed_flag(namespace: &str, item: &Item) -> TemplateParsedFlag {
    let template = TemplateParsedFlag {
        name: item.name.clone(),
        fn_name: format!("{}_{}", namespace, item.name.replace('-', "_").to_lowercase()),
        fn_name: format!("{}_{}", namespace, &item.name),
        is_read_only_enabled: item.permission == Permission::ReadOnly
            && item.state == FlagState::Enabled,
        is_read_only_disabled: item.permission == Permission::ReadOnly
@@ -111,7 +111,7 @@ pub const fn r#test_disabled_ro() -> bool {

#[inline(always)]
pub fn r#test_disabled_rw() -> bool {
    profcollect_libflags_rust::GetServerConfigurableFlag("test", "disabled-rw", "false") == "true"
    profcollect_libflags_rust::GetServerConfigurableFlag("test", "disabled_rw", "false") == "true"
}

#[inline(always)]
@@ -121,7 +121,7 @@ pub const fn r#test_enabled_ro() -> bool {

#[inline(always)]
pub fn r#test_enabled_rw() -> bool {
    profcollect_libflags_rust::GetServerConfigurableFlag("test", "enabled-rw", "false") == "true"
    profcollect_libflags_rust::GetServerConfigurableFlag("test", "enabled_rw", "false") == "true"
}
"#;
        assert_eq!(expected.trim(), String::from_utf8(generated.contents).unwrap().trim());
+1 −0
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ use std::path::{Path, PathBuf};

mod aconfig;
mod cache;
mod codegen;
mod codegen_cpp;
mod codegen_java;
mod codegen_rust;
Loading