From 88712f7610c1c51ee9a58e17ad9f3e1b8425eba9 Mon Sep 17 00:00:00 2001 From: Zhomart Mukhamejanov Date: Tue, 21 Aug 2018 12:19:02 -0700 Subject: [PATCH] Move ab_streaming_metadata under ab_config. Add new config: ab_config.verify_payload_metadata. Change-Id: I521dff92166c33dd9c2efa498dc599fe4bc83fde Signed-off-by: Zhomart Mukhamejanov Test: mmm, junit Bug: 77150191 --- updater_sample/README.md | 2 +- .../systemupdatersample/UpdateConfig.java | 116 ++++++++---------- .../systemupdatersample/UpdateManager.java | 2 +- .../services/PrepareStreamingService.java | 4 +- .../util/UpdateConfigs.java | 4 +- .../res/raw/update_config_001_stream.json | 7 +- .../res/raw/update_config_002_stream.json | 7 +- .../res/raw/update_config_003_nonstream.json | 10 +- .../systemupdatersample/UpdateConfigTest.java | 27 ++-- updater_sample/tools/gen_update_config.py | 37 +++--- .../tools/test_gen_update_config.py | 6 +- 11 files changed, 117 insertions(+), 105 deletions(-) diff --git a/updater_sample/README.md b/updater_sample/README.md index 69e8e244..f9c3fb8e 100644 --- a/updater_sample/README.md +++ b/updater_sample/README.md @@ -220,7 +220,7 @@ privileged system app, so it's granted the required permissions to access - [x] Add Sample app update state (separate from update_engine status) - [x] Add smart update completion detection using onStatusUpdate - [x] Add pause/resume demo -- [ ] Verify system partition checksum for package +- [x] Verify system partition checksum for package ## Running tests diff --git a/updater_sample/src/com/example/android/systemupdatersample/UpdateConfig.java b/updater_sample/src/com/example/android/systemupdatersample/UpdateConfig.java index 1e0fadc2..61872a63 100644 --- a/updater_sample/src/com/example/android/systemupdatersample/UpdateConfig.java +++ b/updater_sample/src/com/example/android/systemupdatersample/UpdateConfig.java @@ -25,6 +25,7 @@ import org.json.JSONObject; import java.io.File; import java.io.Serializable; +import java.util.ArrayList; import java.util.Optional; /** @@ -67,31 +68,28 @@ public class UpdateConfig implements Parcelable { throw new JSONException("Invalid type, expected either " + "NON_STREAMING or STREAMING, got " + o.getString("ab_install_type")); } - if (c.mAbInstallType == AB_INSTALL_TYPE_STREAMING) { - JSONObject meta = o.getJSONObject("ab_streaming_metadata"); - JSONArray propertyFilesJson = meta.getJSONArray("property_files"); - PackageFile[] propertyFiles = - new PackageFile[propertyFilesJson.length()]; - for (int i = 0; i < propertyFilesJson.length(); i++) { - JSONObject p = propertyFilesJson.getJSONObject(i); - propertyFiles[i] = new PackageFile( - p.getString("filename"), - p.getLong("offset"), - p.getLong("size")); - } - String authorization = null; - if (meta.has("authorization")) { - authorization = meta.getString("authorization"); - } - c.mAbStreamingMetadata = new StreamingMetadata( - propertyFiles, - authorization); - } // TODO: parse only for A/B updates when non-A/B is implemented JSONObject ab = o.getJSONObject("ab_config"); boolean forceSwitchSlot = ab.getBoolean("force_switch_slot"); - c.mAbConfig = new AbConfig(forceSwitchSlot); + boolean verifyPayloadMetadata = ab.getBoolean("verify_payload_metadata"); + ArrayList propertyFiles = new ArrayList<>(); + if (ab.has("property_files")) { + JSONArray propertyFilesJson = ab.getJSONArray("property_files"); + for (int i = 0; i < propertyFilesJson.length(); i++) { + JSONObject p = propertyFilesJson.getJSONObject(i); + propertyFiles.add(new PackageFile( + p.getString("filename"), + p.getLong("offset"), + p.getLong("size"))); + } + } + String authorization = ab.optString("authorization", null); + c.mAbConfig = new AbConfig( + forceSwitchSlot, + verifyPayloadMetadata, + propertyFiles.toArray(new PackageFile[0]), + authorization); c.mRawJson = json; return c; @@ -112,9 +110,6 @@ public class UpdateConfig implements Parcelable { /** non-streaming (first saves locally) OR streaming (on the fly) */ private int mAbInstallType; - /** metadata is required only for streaming update */ - private StreamingMetadata mAbStreamingMetadata; - /** A/B update configurations */ private AbConfig mAbConfig; @@ -127,7 +122,6 @@ public class UpdateConfig implements Parcelable { this.mName = in.readString(); this.mUrl = in.readString(); this.mAbInstallType = in.readInt(); - this.mAbStreamingMetadata = (StreamingMetadata) in.readSerializable(); this.mAbConfig = (AbConfig) in.readSerializable(); this.mRawJson = in.readString(); } @@ -154,10 +148,6 @@ public class UpdateConfig implements Parcelable { return mAbInstallType; } - public StreamingMetadata getStreamingMetadata() { - return mAbStreamingMetadata; - } - public AbConfig getAbConfig() { return mAbConfig; } @@ -185,42 +175,10 @@ public class UpdateConfig implements Parcelable { dest.writeString(mName); dest.writeString(mUrl); dest.writeInt(mAbInstallType); - dest.writeSerializable(mAbStreamingMetadata); dest.writeSerializable(mAbConfig); dest.writeString(mRawJson); } - /** - * Metadata for streaming A/B update. - */ - public static class StreamingMetadata implements Serializable { - - private static final long serialVersionUID = 31042L; - - /** defines beginning of update data in archive */ - private PackageFile[] mPropertyFiles; - - /** - * SystemUpdaterSample receives the authorization token from the OTA server, in addition - * to the package URL. It passes on the info to update_engine, so that the latter can - * fetch the data from the package server directly with the token. - */ - private String mAuthorization; - - public StreamingMetadata(PackageFile[] propertyFiles, String authorization) { - this.mPropertyFiles = propertyFiles; - this.mAuthorization = authorization; - } - - public PackageFile[] getPropertyFiles() { - return mPropertyFiles; - } - - public Optional getAuthorization() { - return mAuthorization == null ? Optional.empty() : Optional.of(mAuthorization); - } - } - /** * Description of a file in an OTA package zip file. */ @@ -269,14 +227,48 @@ public class UpdateConfig implements Parcelable { */ private boolean mForceSwitchSlot; - public AbConfig(boolean forceSwitchSlot) { + /** + * if set true device will boot to new slot, otherwise user manually + * switches slot on the screen. + */ + private boolean mVerifyPayloadMetadata; + + /** defines beginning of update data in archive */ + private PackageFile[] mPropertyFiles; + + /** + * SystemUpdaterSample receives the authorization token from the OTA server, in addition + * to the package URL. It passes on the info to update_engine, so that the latter can + * fetch the data from the package server directly with the token. + */ + private String mAuthorization; + + public AbConfig( + boolean forceSwitchSlot, + boolean verifyPayloadMetadata, + PackageFile[] propertyFiles, + String authorization) { this.mForceSwitchSlot = forceSwitchSlot; + this.mVerifyPayloadMetadata = verifyPayloadMetadata; + this.mPropertyFiles = propertyFiles; + this.mAuthorization = authorization; } public boolean getForceSwitchSlot() { return mForceSwitchSlot; } + public boolean getVerifyPayloadMetadata() { + return mVerifyPayloadMetadata; + } + + public PackageFile[] getPropertyFiles() { + return mPropertyFiles; + } + + public Optional getAuthorization() { + return mAuthorization == null ? Optional.empty() : Optional.of(mAuthorization); + } } } diff --git a/updater_sample/src/com/example/android/systemupdatersample/UpdateManager.java b/updater_sample/src/com/example/android/systemupdatersample/UpdateManager.java index a9783e70..12a8f3f5 100644 --- a/updater_sample/src/com/example/android/systemupdatersample/UpdateManager.java +++ b/updater_sample/src/com/example/android/systemupdatersample/UpdateManager.java @@ -324,7 +324,7 @@ public class UpdateManager { if (code == PrepareStreamingService.RESULT_CODE_SUCCESS) { builder.setPayload(payloadSpec); builder.addExtraProperty("USER_AGENT=" + HTTP_USER_AGENT); - config.getStreamingMetadata() + config.getAbConfig() .getAuthorization() .ifPresent(s -> builder.addExtraProperty("AUTHORIZATION=" + s)); updateEngineApplyPayload(builder.build()); diff --git a/updater_sample/src/com/example/android/systemupdatersample/services/PrepareStreamingService.java b/updater_sample/src/com/example/android/systemupdatersample/services/PrepareStreamingService.java index ac6e223e..93140485 100644 --- a/updater_sample/src/com/example/android/systemupdatersample/services/PrepareStreamingService.java +++ b/updater_sample/src/com/example/android/systemupdatersample/services/PrepareStreamingService.java @@ -173,7 +173,7 @@ public class PrepareStreamingService extends IntentService { } /** - * Downloads files defined in {@link UpdateConfig#getStreamingMetadata()} + * Downloads files defined in {@link UpdateConfig#getAbConfig()} * and exists in {@code PRE_STREAMING_FILES_SET}, and put them * in directory {@code dir}. * @throws IOException when can't download a file @@ -185,7 +185,7 @@ public class PrepareStreamingService extends IntentService { Files.deleteIfExists(Paths.get(OTA_PACKAGE_DIR, file)); } Log.d(TAG, "Downloading files to " + dir); - for (UpdateConfig.PackageFile file : config.getStreamingMetadata().getPropertyFiles()) { + for (UpdateConfig.PackageFile file : config.getAbConfig().getPropertyFiles()) { if (PRE_STREAMING_FILES_SET.contains(file.getFilename())) { Log.d(TAG, "Downloading file " + file.getFilename()); FileDownloader downloader = new FileDownloader( diff --git a/updater_sample/src/com/example/android/systemupdatersample/util/UpdateConfigs.java b/updater_sample/src/com/example/android/systemupdatersample/util/UpdateConfigs.java index 5080cb6d..bbefcaf1 100644 --- a/updater_sample/src/com/example/android/systemupdatersample/util/UpdateConfigs.java +++ b/updater_sample/src/com/example/android/systemupdatersample/util/UpdateConfigs.java @@ -83,7 +83,7 @@ public final class UpdateConfigs { /** * @param filename searches by given filename - * @param config searches in {@link UpdateConfig#getStreamingMetadata()} + * @param config searches in {@link UpdateConfig#getAbConfig()} * @return offset and size of {@code filename} in the package zip file * stored as {@link UpdateConfig.PackageFile}. */ @@ -91,7 +91,7 @@ public final class UpdateConfigs { final String filename, UpdateConfig config) { return Arrays - .stream(config.getStreamingMetadata().getPropertyFiles()) + .stream(config.getAbConfig().getPropertyFiles()) .filter(file -> filename.equals(file.getFilename())) .findFirst(); } diff --git a/updater_sample/tests/res/raw/update_config_001_stream.json b/updater_sample/tests/res/raw/update_config_001_stream.json index be51b7c9..b024ad94 100644 --- a/updater_sample/tests/res/raw/update_config_001_stream.json +++ b/updater_sample/tests/res/raw/update_config_001_stream.json @@ -2,7 +2,9 @@ "name": "streaming-001", "url": "http://foo.bar/update.zip", "ab_install_type": "STREAMING", - "ab_streaming_metadata": { + "ab_config": { + "verify_payload_metadata": true, + "force_switch_slot": true, "property_files": [ { "filename": "payload.bin", @@ -10,8 +12,5 @@ "size": 8 } ] - }, - "ab_config": { - "force_switch_slot": true } } diff --git a/updater_sample/tests/res/raw/update_config_002_stream.json b/updater_sample/tests/res/raw/update_config_002_stream.json index 40c8fe1c..12c18bb7 100644 --- a/updater_sample/tests/res/raw/update_config_002_stream.json +++ b/updater_sample/tests/res/raw/update_config_002_stream.json @@ -1,10 +1,8 @@ { "__": "*** Generated using tools/gen_update_config.py ***", "ab_config": { - "force_switch_slot": false - }, - "ab_install_type": "STREAMING", - "ab_streaming_metadata": { + "verify_payload_metadata": true, + "force_switch_slot": false, "property_files": [ { "filename": "payload_metadata.bin", @@ -38,6 +36,7 @@ } ] }, + "ab_install_type": "STREAMING", "name": "S ota_002_package", "url": "file:///data/my-sample-ota-builds-dir/ota_002_package.zip" } \ No newline at end of file diff --git a/updater_sample/tests/res/raw/update_config_003_nonstream.json b/updater_sample/tests/res/raw/update_config_003_nonstream.json index 7c78b9d2..2011f76d 100644 --- a/updater_sample/tests/res/raw/update_config_003_nonstream.json +++ b/updater_sample/tests/res/raw/update_config_003_nonstream.json @@ -1,7 +1,15 @@ { "__": "*** Generated using tools/gen_update_config.py ***", "ab_config": { - "force_switch_slot": false + "verify_payload_metadata": true, + "force_switch_slot": false, + "property_files": [ + { + "filename": "payload.bin", + "offset": 195, + "size": 8 + } + ] }, "ab_install_type": "NON_STREAMING", "name": "S ota_002_package", diff --git a/updater_sample/tests/src/com/example/android/systemupdatersample/UpdateConfigTest.java b/updater_sample/tests/src/com/example/android/systemupdatersample/UpdateConfigTest.java index 1cbd8601..48d0e424 100644 --- a/updater_sample/tests/src/com/example/android/systemupdatersample/UpdateConfigTest.java +++ b/updater_sample/tests/src/com/example/android/systemupdatersample/UpdateConfigTest.java @@ -44,10 +44,12 @@ import java.io.InputStreamReader; @SmallTest public class UpdateConfigTest { - private static final String JSON_NON_STREAMING = - "{\"name\": \"vip update\", \"url\": \"file:///builds/a.zip\", " - + " \"ab_install_type\": \"NON_STREAMING\"," - + " \"ab_config\": { \"force_switch_slot\": false } }"; + private static final String JSON_NON_STREAMING = "{" + + " \"name\": \"vip update\", \"url\": \"file:///my-builds/a.zip\"," + + " \"ab_install_type\": \"NON_STREAMING\"," + + " \"ab_config\": {" + + " \"force_switch_slot\": false," + + " \"verify_payload_metadata\": false } }"; @Rule public final ExpectedException thrown = ExpectedException.none(); @@ -71,7 +73,7 @@ public class UpdateConfigTest { assertSame("type is parsed", UpdateConfig.AB_INSTALL_TYPE_NON_STREAMING, config.getInstallType()); - assertEquals("url is parsed", "file:///builds/a.zip", config.getUrl()); + assertEquals("url is parsed", "file:///my-builds/a.zip", config.getUrl()); } @Test @@ -81,9 +83,9 @@ public class UpdateConfigTest { assertEquals("http://foo.bar/update.zip", config.getUrl()); assertSame(UpdateConfig.AB_INSTALL_TYPE_STREAMING, config.getInstallType()); assertEquals("payload.bin", - config.getStreamingMetadata().getPropertyFiles()[0].getFilename()); - assertEquals(195, config.getStreamingMetadata().getPropertyFiles()[0].getOffset()); - assertEquals(8, config.getStreamingMetadata().getPropertyFiles()[0].getSize()); + config.getAbConfig().getPropertyFiles()[0].getFilename()); + assertEquals(195, config.getAbConfig().getPropertyFiles()[0].getOffset()); + assertEquals(8, config.getAbConfig().getPropertyFiles()[0].getSize()); assertTrue(config.getAbConfig().getForceSwitchSlot()); } @@ -96,9 +98,12 @@ public class UpdateConfigTest { @Test public void getUpdatePackageFile_throwsErrorIfNotAFile() throws Exception { - String json = "{\"name\": \"upd\", \"url\": \"http://foo.bar\"," + String json = "{" + + " \"name\": \"upd\", \"url\": \"http://foo.bar\"," + " \"ab_install_type\": \"NON_STREAMING\"," - + " \"ab_config\": { \"force_switch_slot\": false } }"; + + " \"ab_config\": {" + + " \"force_switch_slot\": false," + + " \"verify_payload_metadata\": false } }"; UpdateConfig config = UpdateConfig.fromJson(json); thrown.expect(RuntimeException.class); config.getUpdatePackageFile(); @@ -107,7 +112,7 @@ public class UpdateConfigTest { @Test public void getUpdatePackageFile_works() throws Exception { UpdateConfig c = UpdateConfig.fromJson(JSON_NON_STREAMING); - assertEquals("/builds/a.zip", c.getUpdatePackageFile().getAbsolutePath()); + assertEquals("/my-builds/a.zip", c.getUpdatePackageFile().getAbsolutePath()); } private String readResource(int id) throws IOException { diff --git a/updater_sample/tools/gen_update_config.py b/updater_sample/tools/gen_update_config.py index b43e49df..f2cb1a8b 100755 --- a/updater_sample/tools/gen_update_config.py +++ b/updater_sample/tools/gen_update_config.py @@ -46,11 +46,17 @@ class GenUpdateConfig(object): AB_INSTALL_TYPE_STREAMING = 'STREAMING' AB_INSTALL_TYPE_NON_STREAMING = 'NON_STREAMING' - def __init__(self, package, url, ab_install_type, ab_force_switch_slot): + def __init__(self, + package, + url, + ab_install_type, + ab_force_switch_slot, + ab_verify_payload_metadata): self.package = package self.url = url self.ab_install_type = ab_install_type self.ab_force_switch_slot = ab_force_switch_slot + self.ab_verify_payload_metadata = ab_verify_payload_metadata self.streaming_required = ( # payload.bin and payload_properties.txt must exist. 'payload.bin', @@ -71,29 +77,24 @@ class GenUpdateConfig(object): def run(self): """Generates config.""" - streaming_metadata = None - if self.ab_install_type == GenUpdateConfig.AB_INSTALL_TYPE_STREAMING: - streaming_metadata = self._gen_ab_streaming_metadata() - self._config = { '__': '*** Generated using tools/gen_update_config.py ***', 'name': self.ab_install_type[0] + ' ' + os.path.basename(self.package)[:-4], 'url': self.url, - 'ab_streaming_metadata': streaming_metadata, + 'ab_config': self._gen_ab_config(), 'ab_install_type': self.ab_install_type, - 'ab_config': { - 'force_switch_slot': self.ab_force_switch_slot, - } } - def _gen_ab_streaming_metadata(self): - """Builds metadata for files required for streaming update.""" + def _gen_ab_config(self): + """Builds config required for A/B update.""" with zipfile.ZipFile(self.package, 'r') as package_zip: - metadata = { - 'property_files': self._get_property_files(package_zip) + config = { + 'property_files': self._get_property_files(package_zip), + 'verify_payload_metadata': self.ab_verify_payload_metadata, + 'force_switch_slot': self.ab_force_switch_slot, } - return metadata + return config @staticmethod def _get_property_files(package_zip): @@ -135,6 +136,11 @@ def main(): # pylint: disable=missing-docstring action='store_true', help='if set device will boot to a new slot, otherwise user ' 'manually switches slot on the screen') + parser.add_argument('--ab_verify_payload_metadata', + default=False, + action='store_true', + help='if set the app will verify the update payload metadata using ' + 'update_engine before downloading the whole package.') parser.add_argument('package', type=str, help='OTA package zip file') @@ -154,7 +160,8 @@ def main(): # pylint: disable=missing-docstring package=args.package, url=args.url, ab_install_type=args.ab_install_type, - ab_force_switch_slot=args.ab_force_switch_slot) + ab_force_switch_slot=args.ab_force_switch_slot, + ab_verify_payload_metadata=args.ab_verify_payload_metadata) gen.run() gen.write(args.out) print('Config is written to ' + args.out) diff --git a/updater_sample/tools/test_gen_update_config.py b/updater_sample/tools/test_gen_update_config.py index c907cf2f..8b77cb2a 100755 --- a/updater_sample/tools/test_gen_update_config.py +++ b/updater_sample/tools/test_gen_update_config.py @@ -32,7 +32,7 @@ class GenUpdateConfigTest(unittest.TestCase): # pylint: disable=missing-docstrin def test_ab_install_type_streaming(self): """tests if streaming property files' offset and size are generated properly""" config, package = self._generate_config() - property_files = config['ab_streaming_metadata']['property_files'] + property_files = config['ab_config']['property_files'] self.assertEqual(len(property_files), 6) with open(package, 'rb') as pkg_file: for prop in property_files: @@ -56,6 +56,8 @@ class GenUpdateConfigTest(unittest.TestCase): # pylint: disable=missing-docstrin '../tests/res/raw/ota_002_package.zip') gen = GenUpdateConfig(ota_package, 'file:///foo.bar', - GenUpdateConfig.AB_INSTALL_TYPE_STREAMING) + GenUpdateConfig.AB_INSTALL_TYPE_STREAMING, + True, # ab_force_switch_slot + True) # ab_verify_payload_metadata gen.run() return gen.config, ota_package