Merge "Close FDs even when throwing SecurityException." into sc-dev (d285109e) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/graphics/fonts/FontManagerService.java

+30 −18

Original line number	Original line	Diff line number	Diff line
	@@ -19,6 +19,7 @@ package com.android.server.graphics.fonts;
	import android.Manifest;		import android.Manifest;
	import android.annotation.NonNull;		import android.annotation.NonNull;
	import android.annotation.Nullable;		import android.annotation.Nullable;
			import android.annotation.RequiresPermission;
	import android.content.Context;		import android.content.Context;
	import android.graphics.Typeface;		import android.graphics.Typeface;
	import android.graphics.fonts.FontFamily;		import android.graphics.fonts.FontFamily;
	@@ -62,6 +63,7 @@ public final class FontManagerService extends IFontManager.Stub {

	private static final String FONT_FILES_DIR = "/data/fonts/files";		private static final String FONT_FILES_DIR = "/data/fonts/files";

			@RequiresPermission(Manifest.permission.UPDATE_FONTS)
	@Override		@Override
	public FontConfig getFontConfig() {		public FontConfig getFontConfig() {
	getContext().enforceCallingPermission(Manifest.permission.UPDATE_FONTS,		getContext().enforceCallingPermission(Manifest.permission.UPDATE_FONTS,
	@@ -69,8 +71,10 @@ public final class FontManagerService extends IFontManager.Stub {
	return getSystemFontConfig();		return getSystemFontConfig();
	}		}

			@RequiresPermission(Manifest.permission.UPDATE_FONTS)
	@Override		@Override
	public int updateFontFamily(@NonNull List<FontUpdateRequest> requests, int baseVersion) {		public int updateFontFamily(@NonNull List<FontUpdateRequest> requests, int baseVersion) {
			try {
	Preconditions.checkArgumentNonnegative(baseVersion);		Preconditions.checkArgumentNonnegative(baseVersion);
	Objects.requireNonNull(requests);		Objects.requireNonNull(requests);
	getContext().enforceCallingPermission(Manifest.permission.UPDATE_FONTS,		getContext().enforceCallingPermission(Manifest.permission.UPDATE_FONTS,
	@@ -81,10 +85,20 @@ public final class FontManagerService extends IFontManager.Stub {
	} catch (SystemFontException e) {		} catch (SystemFontException e) {
	Slog.e(TAG, "Failed to update font family", e);		Slog.e(TAG, "Failed to update font family", e);
	return e.getErrorCode();		return e.getErrorCode();
			}
	} finally {		} finally {
			closeFileDescriptors(requests);
			}
			}

			private static void closeFileDescriptors(@Nullable List<FontUpdateRequest> requests) {
			// Make sure we close every passed FD, even if 'requests' is constructed incorrectly and
			// some fields are null.
			if (requests == null) return;
	for (FontUpdateRequest request : requests) {		for (FontUpdateRequest request : requests) {
			if (request == null) continue;
	ParcelFileDescriptor fd = request.getFd();		ParcelFileDescriptor fd = request.getFd();
	if (fd != null) {		if (fd == null) continue;
	try {		try {
	fd.close();		fd.close();
	} catch (IOException e) {		} catch (IOException e) {
	@@ -92,8 +106,6 @@ public final class FontManagerService extends IFontManager.Stub {
	}		}
	}		}
	}		}
	}
	}

	/* package */ static class SystemFontException extends AndroidException {		/* package */ static class SystemFontException extends AndroidException {
	private final int mErrorCode;		private final int mErrorCode;

tests/UpdatableSystemFontTest/src/com/android/updatablesystemfont/UpdatableSystemFontTest.java

+26 −7

Original line number	Original line	Diff line number	Diff line
	@@ -20,6 +20,7 @@ import static android.os.ParcelFileDescriptor.MODE_READ_ONLY;

	import static com.google.common.truth.Truth.assertThat;		import static com.google.common.truth.Truth.assertThat;

			import static org.junit.Assert.assertThrows;
	import static org.junit.Assume.assumeTrue;		import static org.junit.Assume.assumeTrue;

	import static java.util.concurrent.TimeUnit.SECONDS;		import static java.util.concurrent.TimeUnit.SECONDS;
	@@ -247,7 +248,6 @@ public class UpdatableSystemFontTest {
	assertThat(fontPathAfterReboot).isEqualTo(fontPath);		assertThat(fontPathAfterReboot).isEqualTo(fontPath);
	}		}


	@Test		@Test
	public void fdLeakTest() throws Exception {		public void fdLeakTest() throws Exception {
	long originalOpenFontCount =		long originalOpenFontCount =
	@@ -273,6 +273,20 @@ public class UpdatableSystemFontTest {
	}		}
	}		}

			@Test
			public void fdLeakTest_withoutPermission() throws Exception {
			Pattern patternEmojiVPlus1 =
			Pattern.compile(Pattern.quote(TEST_NOTO_COLOR_EMOJI_VPLUS1_TTF));
			byte[] signature = Files.readAllBytes(Paths.get(TEST_NOTO_COLOR_EMOJI_VPLUS1_TTF_FSV_SIG));
			try (ParcelFileDescriptor fd = ParcelFileDescriptor.open(
			new File(TEST_NOTO_COLOR_EMOJI_VPLUS1_TTF), MODE_READ_ONLY)) {
			assertThrows(SecurityException.class,
			() -> updateFontFileWithoutPermission(fd, signature, 0));
			}
			List<String> openFiles = getOpenFiles("system_server");
			assertThat(countMatch(openFiles, patternEmojiVPlus1)).isEqualTo(0);
			}

	private static String insertCert(String certPath) throws Exception {		private static String insertCert(String certPath) throws Exception {
	Pair<String, String> result;		Pair<String, String> result;
	try (InputStream is = new FileInputStream(certPath)) {		try (InputStream is = new FileInputStream(certPath)) {
	@@ -290,14 +304,19 @@ public class UpdatableSystemFontTest {
	try (ParcelFileDescriptor fd =		try (ParcelFileDescriptor fd =
	ParcelFileDescriptor.open(new File(fontPath), MODE_READ_ONLY)) {		ParcelFileDescriptor.open(new File(fontPath), MODE_READ_ONLY)) {
	return SystemUtil.runWithShellPermissionIdentity(() -> {		return SystemUtil.runWithShellPermissionIdentity(() -> {
	FontConfig fontConfig = mFontManager.getFontConfig();		int configVersion = mFontManager.getFontConfig().getConfigVersion();
			return updateFontFileWithoutPermission(fd, signature, configVersion);
			});
			}
			}

			private int updateFontFileWithoutPermission(ParcelFileDescriptor fd, byte[] signature,
			int configVersion) {
	return mFontManager.updateFontFamily(		return mFontManager.updateFontFamily(
	new FontFamilyUpdateRequest.Builder()		new FontFamilyUpdateRequest.Builder()
	.addFontFileUpdateRequest(new FontFileUpdateRequest(fd, signature))		.addFontFileUpdateRequest(new FontFileUpdateRequest(fd, signature))
	.build(),		.build(),
	fontConfig.getConfigVersion());		configVersion);
	});
	}
	}		}

	private String getFontPath(String psName) {		private String getFontPath(String psName) {