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

Commit cae5560e authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Android (Google) Code Review
Browse files

Merge "Create XML parser only once."

parents 5f463315 0c39dd02

core/java/com/android/server/SystemConfig.java

+26 −23
Original line number Diff line number Diff line
@@ -488,12 +488,14 @@ public class SystemConfig { 
    }



    private void readAllPermissions() {

        final XmlPullParser parser = Xml.newPullParser();



        // Read configuration from system

        readPermissions(Environment.buildPath(

        readPermissions(parser, Environment.buildPath(

                Environment.getRootDirectory(), "etc", "sysconfig"), ALLOW_ALL);



        // Read configuration from the old permissions dir

        readPermissions(Environment.buildPath(

        readPermissions(parser, Environment.buildPath(

                Environment.getRootDirectory(), "etc", "permissions"), ALLOW_ALL);



        // Vendors are only allowed to customize these
@@ -503,18 +505,18 @@ public class SystemConfig { 
            // For backward compatibility

            vendorPermissionFlag |= (ALLOW_PERMISSIONS | ALLOW_APP_CONFIGS);

        }

        readPermissions(Environment.buildPath(

        readPermissions(parser, Environment.buildPath(

                Environment.getVendorDirectory(), "etc", "sysconfig"), vendorPermissionFlag);

        readPermissions(Environment.buildPath(

        readPermissions(parser, Environment.buildPath(

                Environment.getVendorDirectory(), "etc", "permissions"), vendorPermissionFlag);



        String vendorSkuProperty = SystemProperties.get(VENDOR_SKU_PROPERTY, "");

        if (!vendorSkuProperty.isEmpty()) {

            String vendorSkuDir = "sku_" + vendorSkuProperty;

            readPermissions(Environment.buildPath(

            readPermissions(parser, Environment.buildPath(

                    Environment.getVendorDirectory(), "etc", "sysconfig", vendorSkuDir),

                    vendorPermissionFlag);

            readPermissions(Environment.buildPath(

            readPermissions(parser, Environment.buildPath(

                    Environment.getVendorDirectory(), "etc", "permissions", vendorSkuDir),

                    vendorPermissionFlag);

        }
@@ -522,18 +524,18 @@ public class SystemConfig { 
        // Allow ODM to customize system configs as much as Vendor, because /odm is another

        // vendor partition other than /vendor.

        int odmPermissionFlag = vendorPermissionFlag;

        readPermissions(Environment.buildPath(

        readPermissions(parser, Environment.buildPath(

                Environment.getOdmDirectory(), "etc", "sysconfig"), odmPermissionFlag);

        readPermissions(Environment.buildPath(

        readPermissions(parser, Environment.buildPath(

                Environment.getOdmDirectory(), "etc", "permissions"), odmPermissionFlag);



        String skuProperty = SystemProperties.get(SKU_PROPERTY, "");

        if (!skuProperty.isEmpty()) {

            String skuDir = "sku_" + skuProperty;



            readPermissions(Environment.buildPath(

            readPermissions(parser, Environment.buildPath(

                    Environment.getOdmDirectory(), "etc", "sysconfig", skuDir), odmPermissionFlag);

            readPermissions(Environment.buildPath(

            readPermissions(parser, Environment.buildPath(

                    Environment.getOdmDirectory(), "etc", "permissions", skuDir),

                    odmPermissionFlag);

        }
@@ -541,9 +543,9 @@ public class SystemConfig { 
        // Allow OEM to customize these

        int oemPermissionFlag = ALLOW_FEATURES | ALLOW_OEM_PERMISSIONS | ALLOW_ASSOCIATIONS

                | ALLOW_VENDOR_APEX;

        readPermissions(Environment.buildPath(

        readPermissions(parser, Environment.buildPath(

                Environment.getOemDirectory(), "etc", "sysconfig"), oemPermissionFlag);

        readPermissions(Environment.buildPath(

        readPermissions(parser, Environment.buildPath(

                Environment.getOemDirectory(), "etc", "permissions"), oemPermissionFlag);



        // Allow Product to customize these configs
@@ -558,15 +560,15 @@ public class SystemConfig { 
            // DEVICE_INITIAL_SDK_INT for the devices without product interface enforcement.

            productPermissionFlag = ALLOW_ALL;

        }

        readPermissions(Environment.buildPath(

        readPermissions(parser, Environment.buildPath(

                Environment.getProductDirectory(), "etc", "sysconfig"), productPermissionFlag);

        readPermissions(Environment.buildPath(

        readPermissions(parser, Environment.buildPath(

                Environment.getProductDirectory(), "etc", "permissions"), productPermissionFlag);



        // Allow /system_ext to customize all system configs

        readPermissions(Environment.buildPath(

        readPermissions(parser, Environment.buildPath(

                Environment.getSystemExtDirectory(), "etc", "sysconfig"), ALLOW_ALL);

        readPermissions(Environment.buildPath(

        readPermissions(parser, Environment.buildPath(

                Environment.getSystemExtDirectory(), "etc", "permissions"), ALLOW_ALL);



        // Skip loading configuration from apex if it is not a system process.
@@ -580,12 +582,13 @@ public class SystemConfig { 
            if (f.isFile() || f.getPath().contains("@")) {

                continue;

            }

            readPermissions(Environment.buildPath(f, "etc", "permissions"), apexPermissionFlag);

            readPermissions(parser, Environment.buildPath(f, "etc", "permissions"),

                    apexPermissionFlag);

        }

    }



    @VisibleForTesting

    public void readPermissions(File libraryDir, int permissionFlag) {

    public void readPermissions(final XmlPullParser parser, File libraryDir, int permissionFlag) {

        // Read permissions from given directory.

        if (!libraryDir.exists() || !libraryDir.isDirectory()) {

            if (permissionFlag == ALLOW_ALL) {
@@ -620,12 +623,12 @@ public class SystemConfig { 
                continue;

            }



            readPermissionsFromXml(f, permissionFlag);

            readPermissionsFromXml(parser, f, permissionFlag);

        }



        // Read platform permissions last so it will take precedence

        if (platformFile != null) {

            readPermissionsFromXml(platformFile, permissionFlag);

            readPermissionsFromXml(parser, platformFile, permissionFlag);

        }

    }
@@ -634,8 +637,9 @@ public class SystemConfig { 
                + permFile + " at " + parser.getPositionDescription());

    }



    private void readPermissionsFromXml(File permFile, int permissionFlag) {

        FileReader permReader = null;

    private void readPermissionsFromXml(final XmlPullParser parser, File permFile,

            int permissionFlag) {

        final FileReader permReader;

        try {

            permReader = new FileReader(permFile);

        } catch (FileNotFoundException e) {
@@ -647,7 +651,6 @@ public class SystemConfig { 
        final boolean lowRam = ActivityManager.isLowRamDeviceStatic();



        try {

            XmlPullParser parser = Xml.newPullParser();

            parser.setInput(permReader);



            int type;

services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigNamedActorTest.kt

+3 −1
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package com.android.server.systemconfig



import android.content.Context

import android.util.Xml

import androidx.test.InstrumentationRegistry

import com.android.server.SystemConfig

import com.google.common.truth.Truth.assertThat
@@ -227,6 +228,7 @@ class SystemConfigNamedActorTest { 
            .writeText(this.trimIndent())



    private fun assertPermissions() = SystemConfig(false).apply {

        readPermissions(tempFolder.root, 0)

        val parser = Xml.newPullParser()

        readPermissions(parser, tempFolder.root, 0)

    }. let { assertThat(it.namedActors) }

}

services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java

+17 −10
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import android.platform.test.annotations.Presubmit; 
import android.util.ArrayMap;

import android.util.ArraySet;

import android.util.Log;

import android.util.Xml;



import androidx.test.filters.SmallTest;

import androidx.test.runner.AndroidJUnit4;
@@ -36,6 +37,7 @@ import org.junit.Rule; 
import org.junit.Test;

import org.junit.rules.TemporaryFolder;

import org.junit.runner.RunWith;

import org.xmlpull.v1.XmlPullParser;



import java.io.BufferedWriter;

import java.io.File;
@@ -76,6 +78,11 @@ public class SystemConfigTest { 
        }

    }



    private void readPermissions(File libraryDir, int permissionFlag) {

        final XmlPullParser parser = Xml.newPullParser();

        mSysConfig.readPermissions(parser, libraryDir, permissionFlag);

    }



    /**

     * Tests that readPermissions works correctly for the tag: install-in-user-type

     */
@@ -134,8 +141,8 @@ public class SystemConfigTest { 
        // Also, make a third file, but with the name folder1/permFile2.xml, to prove no conflicts.

        createTempFile(folder1, "permFile2.xml", contents3);



        mSysConfig.readPermissions(folder1, /* No permission needed anyway */ 0);

        mSysConfig.readPermissions(folder2, /* No permission needed anyway */ 0);

        readPermissions(folder1, /* No permission needed anyway */ 0);

        readPermissions(folder2, /* No permission needed anyway */ 0);



        Map<String, Set<String>> actualWhite = mSysConfig.getAndClearPackageToUserTypeWhitelist();

        Map<String, Set<String>> actualBlack = mSysConfig.getAndClearPackageToUserTypeBlacklist();
@@ -165,7 +172,7 @@ public class SystemConfigTest { 
        final File folder = createTempSubfolder("folder");

        createTempFile(folder, "component-override.xml", contents);



        mSysConfig.readPermissions(folder, /* No permission needed anyway */ 0);

        readPermissions(folder, /* No permission needed anyway */ 0);



        final ArrayMap<String, Boolean> packageOneExpected = new ArrayMap<>();

        packageOneExpected.put("com.android.package1.Full", true);
@@ -197,7 +204,7 @@ public class SystemConfigTest { 
        final File folder = createTempSubfolder("folder");

        createTempFile(folder, "staged-installer-whitelist.xml", contents);



        mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0);

        readPermissions(folder, /* Grant all permission flags */ ~0);



        assertThat(mSysConfig.getWhitelistedStagedInstallers())

                .containsExactly("com.android.package1");
@@ -215,7 +222,7 @@ public class SystemConfigTest { 
        final File folder = createTempSubfolder("folder");

        createTempFile(folder, "staged-installer-whitelist.xml", contents);



        mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0);

        readPermissions(folder, /* Grant all permission flags */ ~0);



        assertThat(mSysConfig.getWhitelistedStagedInstallers())

                .containsExactly("com.android.package1");
@@ -238,7 +245,7 @@ public class SystemConfigTest { 


        IllegalStateException e = expectThrows(

                IllegalStateException.class,

                () -> mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0));

                () -> readPermissions(folder, /* Grant all permission flags */ ~0));



        assertThat(e).hasMessageThat().contains("Multiple modules installers");

    }
@@ -257,7 +264,7 @@ public class SystemConfigTest { 
        final File folder = createTempSubfolder("folder");

        createTempFile(folder, "staged-installer-whitelist.xml", contents);



        mSysConfig.readPermissions(folder, /* Grant all but ALLOW_APP_CONFIGS flag */ ~0x08);

        readPermissions(folder, /* Grant all but ALLOW_APP_CONFIGS flag */ ~0x08);



        assertThat(mSysConfig.getWhitelistedStagedInstallers()).isEmpty();

    }
@@ -277,7 +284,7 @@ public class SystemConfigTest { 
        final File folder = createTempSubfolder("folder");

        createTempFile(folder, "vendor-apex-allowlist.xml", contents);



        mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0);

        readPermissions(folder, /* Grant all permission flags */ ~0);



        assertThat(mSysConfig.getAllowedVendorApexes())

                .containsExactly("com.android.apex1", "com.installer");
@@ -297,7 +304,7 @@ public class SystemConfigTest { 
        final File folder = createTempSubfolder("folder");

        createTempFile(folder, "vendor-apex-allowlist.xml", contents);



        mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0);

        readPermissions(folder, /* Grant all permission flags */ ~0);



        assertThat(mSysConfig.getAllowedVendorApexes()).isEmpty();

    }
@@ -317,7 +324,7 @@ public class SystemConfigTest { 
        final File folder = createTempSubfolder("folder");

        createTempFile(folder, "vendor-apex-allowlist.xml", contents);



        mSysConfig.readPermissions(folder, /* Grant all but ALLOW_VENDOR_APEX flag */ ~0x400);

        readPermissions(folder, /* Grant all but ALLOW_VENDOR_APEX flag */ ~0x400);



        assertThat(mSysConfig.getAllowedVendorApexes()).isEmpty();

    }