From 61aeac6dc578026f3928d6653d43c791d00172d2 Mon Sep 17 00:00:00 2001 From: Winson Date: Wed, 3 Mar 2021 12:48:40 -0800 Subject: [PATCH] Do not force endDocument for domain verify settings Removes the endDocument call so the serializer can be used as a child of a larger group. Instead calls flush and assumes all usages will close tags appropriately. This usage is incorrect regardless, but endDocument itself is broken and does not behave the same depending on which XmlSerialier implementation is being used. Which is why this bug never appeared during testing/development. Bug: 180838875 Bug: 181813200 Test: manual, debug linked Bug Test: atest DomainVerificationPersistenceTest Change-Id: I50e27817e526d2cbcc0efbcc3ffdfffa53886f0f --- .../com/android/server/pm/SettingsXml.java | 2 +- .../DomainVerificationLegacySettings.java | 1 - .../DomainVerificationPersistenceTest.kt | 27 ++++++++++++++++--- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/pm/SettingsXml.java b/services/core/java/com/android/server/pm/SettingsXml.java index ec643f5980417..c53fef72db9cc 100644 --- a/services/core/java/com/android/server/pm/SettingsXml.java +++ b/services/core/java/com/android/server/pm/SettingsXml.java @@ -83,7 +83,7 @@ public class SettingsXml { @Override public void close() throws IOException { mWriteSection.closeCompletely(); - mXmlSerializer.endDocument(); + mXmlSerializer.flush(); } } diff --git a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationLegacySettings.java b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationLegacySettings.java index c787356f342c9..4bad1020e9452 100644 --- a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationLegacySettings.java +++ b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationLegacySettings.java @@ -32,7 +32,6 @@ import com.android.server.pm.SettingsXml; import org.xmlpull.v1.XmlPullParserException; import java.io.IOException; -import java.util.Map; /** * Reads and writes the old {@link android.content.pm.IntentFilterVerificationInfo} so that it can diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPersistenceTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPersistenceTest.kt index a92ab9e35ddca..f8a3fb94bd3e3 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPersistenceTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPersistenceTest.kt @@ -30,6 +30,7 @@ import com.google.common.truth.Truth.assertWithMessage import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder +import org.xmlpull.v1.XmlPullParser import java.io.File import java.nio.charset.StandardCharsets import java.util.UUID @@ -41,21 +42,41 @@ class DomainVerificationPersistenceTest { internal fun File.writeXml(block: (serializer: TypedXmlSerializer) -> Unit) = apply { outputStream().use { - // Explicitly use string based XML so it can printed in the test failure output - Xml.newFastSerializer() + // This must use the binary serializer the mirror the production behavior, as + // there are slight differences with the string based one. + Xml.newBinarySerializer() .apply { setOutput(it, StandardCharsets.UTF_8.name()) startDocument(null, true) setFeature("http://xmlpull.org/v1/doc/features.html#indent-output", true) + // Write a wrapping tag to ensure the domain verification settings didn't + // close out the document, allowing other settings to be written + startTag(null, "wrapper-tag") } .apply(block) + .apply { + startTag(null, "trailing-tag") + endTag(null, "trailing-tag") + endTag(null, "wrapper-tag") + } .endDocument() } } internal fun File.readXml(block: (parser: TypedXmlPullParser) -> T) = inputStream().use { - block(Xml.resolvePullParser(it)) + val parser = Xml.resolvePullParser(it) + assertThat(parser.nextTag()).isEqualTo(XmlPullParser.START_TAG) + assertThat(parser.name).isEqualTo("wrapper-tag") + assertThat(parser.nextTag()).isEqualTo(XmlPullParser.START_TAG) + block(parser).also { + assertThat(parser.nextTag()).isEqualTo(XmlPullParser.START_TAG) + assertThat(parser.name).isEqualTo("trailing-tag") + assertThat(parser.nextTag()).isEqualTo(XmlPullParser.END_TAG) + assertThat(parser.name).isEqualTo("trailing-tag") + assertThat(parser.nextTag()).isEqualTo(XmlPullParser.END_TAG) + assertThat(parser.name).isEqualTo("wrapper-tag") + } } }