From 7b7f417d92431c93244102725680bbbe2d840b0b Mon Sep 17 00:00:00 2001 From: Kohsuke Yatoh Date: Thu, 29 Apr 2021 15:34:24 -0700 Subject: [PATCH] Use AtomicFile in UpdatableFontDir. Also add more precondition checks. Bug: 186785560 Test: atest FrameworksServicesTests:UpdatableFontDirTest Test: atest UpdatableSystemFontTest Change-Id: I4edaf1d2f6dcc9de1f0b1adc7db4a83a76d5f2a2 --- .../graphics/fonts/FontUpdateRequest.java | 7 +- .../graphics/fonts/UpdatableFontDir.java | 91 +++++++++++-------- .../graphics/fonts/UpdatableFontDirTest.java | 13 +-- 3 files changed, 63 insertions(+), 48 deletions(-) diff --git a/core/java/android/graphics/fonts/FontUpdateRequest.java b/core/java/android/graphics/fonts/FontUpdateRequest.java index 4dd5a72d446e4..cda1638a24dcb 100644 --- a/core/java/android/graphics/fonts/FontUpdateRequest.java +++ b/core/java/android/graphics/fonts/FontUpdateRequest.java @@ -147,7 +147,7 @@ public final class FontUpdateRequest implements Parcelable { public static Font readFromXml(XmlPullParser parser) throws IOException { String psName = parser.getAttributeValue(null, ATTR_POSTSCRIPT_NAME); if (psName == null) { - throw new IOException("name attribute is missing font tag."); + throw new IOException("name attribute is missing in font tag."); } int index = getAttributeValueInt(parser, ATTR_INDEX, 0); int weight = getAttributeValueInt(parser, ATTR_WEIGHT, FontStyle.FONT_WEIGHT_NORMAL); @@ -210,7 +210,7 @@ public final class FontUpdateRequest implements Parcelable { private static final String ATTR_NAME = "name"; private static final String TAG_FONT = "font"; - private final @Nullable String mName; + private final @NonNull String mName; private final @NonNull List mFonts; public Family(String name, List fonts) { @@ -281,6 +281,9 @@ public final class FontUpdateRequest implements Parcelable { throw new IOException("Unexpected parser state: must be START_TAG with family"); } String name = parser.getAttributeValue(null, ATTR_NAME); + if (name == null) { + throw new IOException("name attribute is missing in family tag."); + } int type = 0; while ((type = parser.next()) != XmlPullParser.END_DOCUMENT) { if (type == XmlPullParser.START_TAG && parser.getName().equals(TAG_FONT)) { diff --git a/services/core/java/com/android/server/graphics/fonts/UpdatableFontDir.java b/services/core/java/com/android/server/graphics/fonts/UpdatableFontDir.java index 38d1aab705509..06a9832f1b544 100644 --- a/services/core/java/com/android/server/graphics/fonts/UpdatableFontDir.java +++ b/services/core/java/com/android/server/graphics/fonts/UpdatableFontDir.java @@ -28,6 +28,7 @@ import android.system.ErrnoException; import android.system.Os; import android.text.FontConfig; import android.util.ArrayMap; +import android.util.AtomicFile; import android.util.Base64; import android.util.Slog; @@ -43,6 +44,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.function.Function; import java.util.function.Supplier; @@ -120,8 +122,7 @@ final class UpdatableFontDir { private final File mFilesDir; private final FontFileParser mParser; private final FsverityUtil mFsverityUtil; - private final File mConfigFile; - private final File mTmpConfigFile; + private final AtomicFile mConfigFile; private final Supplier mCurrentTimeSupplier; private final Function, FontConfig> mConfigSupplier; @@ -131,13 +132,13 @@ final class UpdatableFontDir { /** * A mutable map containing mapping from font file name (e.g. "NotoColorEmoji.ttf") to {@link * FontFileInfo}. All files in this map are validated, and have higher revision numbers than - * corresponding font files in {@link #mPreinstalledFontDirs}. + * corresponding font files returned by {@link #mConfigSupplier}. */ private final ArrayMap mFontFileInfoMap = new ArrayMap<>(); UpdatableFontDir(File filesDir, FontFileParser parser, FsverityUtil fsverityUtil) { this(filesDir, parser, fsverityUtil, new File(CONFIG_XML_FILE), - () -> System.currentTimeMillis(), + System::currentTimeMillis, (map) -> SystemFonts.getSystemFontConfig(map, 0, 0) ); } @@ -149,8 +150,7 @@ final class UpdatableFontDir { mFilesDir = filesDir; mParser = parser; mFsverityUtil = fsverityUtil; - mConfigFile = configFile; - mTmpConfigFile = new File(configFile.getAbsoluteFile() + ".tmp"); + mConfigFile = new AtomicFile(configFile); mCurrentTimeSupplier = currentTimeSupplier; mConfigSupplier = configSupplier; } @@ -166,18 +166,16 @@ final class UpdatableFontDir { mConfigVersion = 1; boolean success = false; try { - PersistentSystemFontConfig.Config config = new PersistentSystemFontConfig.Config(); - try (FileInputStream fis = new FileInputStream(mConfigFile)) { - PersistentSystemFontConfig.loadFromXml(fis, config); - } catch (IOException | XmlPullParserException e) { - // The font config file is missing on the first boot. Just do nothing. - return; - } + PersistentSystemFontConfig.Config config = readPersistentConfig(); mLastModifiedMillis = config.lastModifiedMillis; File[] dirs = mFilesDir.listFiles(); - if (dirs == null) return; - FontConfig fontConfig = getSystemFontConfig(); + if (dirs == null) { + // mFilesDir should be created by init script. + Slog.e(TAG, "Could not read: " + mFilesDir); + return; + } + FontConfig fontConfig = null; for (File dir : dirs) { if (!dir.getName().startsWith(RANDOM_DIR_PREFIX)) { Slog.e(TAG, "Unexpected dir found: " + dir); @@ -194,6 +192,9 @@ final class UpdatableFontDir { return; } FontFileInfo fontFileInfo = validateFontFile(files[0]); + if (fontConfig == null) { + fontConfig = getSystemFontConfig(); + } addFileToMapIfSameOrNewer(fontFileInfo, fontConfig, true /* deleteOldFile */); } success = true; @@ -216,15 +217,9 @@ final class UpdatableFontDir { FileUtils.deleteContents(mFilesDir); mLastModifiedMillis = mCurrentTimeSupplier.get(); - try (FileOutputStream fos = new FileOutputStream(mConfigFile)) { - PersistentSystemFontConfig.Config config = new PersistentSystemFontConfig.Config(); - config.lastModifiedMillis = mLastModifiedMillis; - PersistentSystemFontConfig.writeToXml(fos, config); - } catch (Exception e) { - throw new SystemFontException( - FontManager.RESULT_ERROR_FAILED_UPDATE_CONFIG, - "Failed to write config XML.", e); - } + PersistentSystemFontConfig.Config config = new PersistentSystemFontConfig.Config(); + config.lastModifiedMillis = mLastModifiedMillis; + writePersistentConfig(config); mConfigVersion++; } @@ -234,6 +229,18 @@ final class UpdatableFontDir { * before this method is called. */ public void update(List requests) throws SystemFontException { + for (FontUpdateRequest request : requests) { + switch (request.getType()) { + case FontUpdateRequest.TYPE_UPDATE_FONT_FILE: + Objects.requireNonNull(request.getFd()); + Objects.requireNonNull(request.getSignature()); + break; + case FontUpdateRequest.TYPE_UPDATE_FONT_FAMILY: + Objects.requireNonNull(request.getFontFamily()); + Objects.requireNonNull(request.getFontFamily().getName()); + break; + } + } // Backup the mapping for rollback. ArrayMap backupMap = new ArrayMap<>(mFontFileInfoMap); PersistentSystemFontConfig.Config curConfig = readPersistentConfig(); @@ -277,20 +284,7 @@ final class UpdatableFontDir { newConfig.updatedFontDirs.add(info.getRandomizedFontDir().getName()); } newConfig.fontFamilies.addAll(familyMap.values()); - - try (FileOutputStream fos = new FileOutputStream(mTmpConfigFile)) { - PersistentSystemFontConfig.writeToXml(fos, newConfig); - } catch (Exception e) { - throw new SystemFontException( - FontManager.RESULT_ERROR_FAILED_UPDATE_CONFIG, - "Failed to write config XML.", e); - } - - if (!mFsverityUtil.rename(mTmpConfigFile, mConfigFile)) { - throw new SystemFontException( - FontManager.RESULT_ERROR_FAILED_UPDATE_CONFIG, - "Failed to stage the config file."); - } + writePersistentConfig(newConfig); mConfigVersion++; success = true; } finally { @@ -420,7 +414,7 @@ final class UpdatableFontDir { /** * Add the given {@link FontFileInfo} to {@link #mFontFileInfoMap} if its font revision is * equal to or higher than the revision of currently used font file (either in - * {@link #mFontFileInfoMap} or {@link #mPreinstalledFontDirs}). + * {@link #mFontFileInfoMap} or {@code fontConfig}). */ private boolean addFileToMapIfSameOrNewer(FontFileInfo fontFileInfo, FontConfig fontConfig, boolean deleteOldFile) { @@ -568,7 +562,7 @@ final class UpdatableFontDir { private PersistentSystemFontConfig.Config readPersistentConfig() { PersistentSystemFontConfig.Config config = new PersistentSystemFontConfig.Config(); - try (FileInputStream fis = new FileInputStream(mConfigFile)) { + try (FileInputStream fis = mConfigFile.openRead()) { PersistentSystemFontConfig.loadFromXml(fis, config); } catch (IOException | XmlPullParserException e) { // The font config file is missing on the first boot. Just do nothing. @@ -576,6 +570,23 @@ final class UpdatableFontDir { return config; } + private void writePersistentConfig(PersistentSystemFontConfig.Config config) + throws SystemFontException { + FileOutputStream fos = null; + try { + fos = mConfigFile.startWrite(); + PersistentSystemFontConfig.writeToXml(fos, config); + mConfigFile.finishWrite(fos); + } catch (IOException e) { + if (fos != null) { + mConfigFile.failWrite(fos); + } + throw new SystemFontException( + FontManager.RESULT_ERROR_FAILED_UPDATE_CONFIG, + "Failed to write config XML.", e); + } + } + /* package */ int getConfigVersion() { return mConfigVersion; } diff --git a/services/tests/servicestests/src/com/android/server/graphics/fonts/UpdatableFontDirTest.java b/services/tests/servicestests/src/com/android/server/graphics/fonts/UpdatableFontDirTest.java index 765c13a98ac06..7b4803724180c 100644 --- a/services/tests/servicestests/src/com/android/server/graphics/fonts/UpdatableFontDirTest.java +++ b/services/tests/servicestests/src/com/android/server/graphics/fonts/UpdatableFontDirTest.java @@ -857,14 +857,15 @@ public final class UpdatableFontDirTest { mConfigFile, mCurrentTimeSupplier, mConfigSupplier); dir.loadFontFileMap(); + List requests = Arrays.asList( + newFontUpdateRequest("test.ttf,1,test", GOOD_SIGNATURE), + newAddFontFamilyRequest("" + + " test.ttf" + + "")); try { - dir.update(Arrays.asList( - newFontUpdateRequest("test.ttf,1,test", GOOD_SIGNATURE), - newAddFontFamilyRequest("" - + " test.ttf" - + ""))); + dir.update(requests); fail("Expect NullPointerException"); - } catch (FontManagerService.SystemFontException e) { + } catch (NullPointerException e) { // Expect } }