From 25469aacb8fa4f0198af945b5a9878008e16f2c4 Mon Sep 17 00:00:00 2001 From: Sudheer Shanka Date: Mon, 27 Aug 2018 15:50:23 -0700 Subject: Remove DefaultContainerService usage in StorageManagerService. StorageManagerService uses DefaultContainerService to obtain ObbInfo for files passed through mountObb() transaction. This change moves this logic to client side and so ObbInfo will be passed as part of mountObb() transaction. Bug: 111838160 Test: atest src/android/os/storage/cts/StorageManagerTest.java Test: atest core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java Test: atest services/tests/servicestests/src/com/android/server/MountServiceTests.java Change-Id: I29aee3aa54a45057df96aae289888161a3e3af71 --- core/java/android/content/res/ObbInfo.java | 1 + core/java/android/os/storage/IStorageManager.aidl | 3 +- core/java/android/os/storage/StorageManager.java | 14 +- .../android/os/storage/StorageManagerBaseTest.java | 95 +---------- .../os/storage/StorageManagerIntegrationTest.java | 52 +----- libs/storage/Android.bp | 1 + libs/storage/IMountService.cpp | 3 +- libs/storage/ObbInfo.cpp | 47 ++++++ libs/storage/include/storage/IMountService.h | 3 +- libs/storage/include/storage/ObbInfo.h | 48 ++++++ native/android/storage_manager.cpp | 26 ++- .../com/android/server/StorageManagerService.java | 185 ++------------------- .../src/com/android/server/MountServiceTests.java | 7 +- 13 files changed, 170 insertions(+), 315 deletions(-) create mode 100644 libs/storage/ObbInfo.cpp create mode 100644 libs/storage/include/storage/ObbInfo.h diff --git a/core/java/android/content/res/ObbInfo.java b/core/java/android/content/res/ObbInfo.java index 452fdce6f031..1d10b4fef45c 100644 --- a/core/java/android/content/res/ObbInfo.java +++ b/core/java/android/content/res/ObbInfo.java @@ -80,6 +80,7 @@ public class ObbInfo implements Parcelable { } public void writeToParcel(Parcel dest, int parcelableFlags) { + // Keep this in sync with writeToParcel() in ObbInfo.cpp dest.writeString(filename); dest.writeString(packageName); dest.writeInt(version); diff --git a/core/java/android/os/storage/IStorageManager.aidl b/core/java/android/os/storage/IStorageManager.aidl index 5c2d41e1aebc..33b267600c1d 100644 --- a/core/java/android/os/storage/IStorageManager.aidl +++ b/core/java/android/os/storage/IStorageManager.aidl @@ -17,6 +17,7 @@ package android.os.storage; import android.content.pm.IPackageMoveObserver; +import android.content.res.ObbInfo; import android.os.IVoldTaskListener; import android.os.ParcelFileDescriptor; import android.os.storage.DiskInfo; @@ -57,7 +58,7 @@ interface IStorageManager { * it of the terminal state of the call. */ void mountObb(in String rawPath, in String canonicalPath, in String key, - IObbActionListener token, int nonce) = 21; + IObbActionListener token, int nonce, in ObbInfo obbInfo) = 21; /** * Unmounts an Opaque Binary Blob (OBB). When the force flag is specified, * any program using it will be forcibly killed to unmount the image. diff --git a/core/java/android/os/storage/StorageManager.java b/core/java/android/os/storage/StorageManager.java index 5a1ea68b65da..e69acaed1ffe 100644 --- a/core/java/android/os/storage/StorageManager.java +++ b/core/java/android/os/storage/StorageManager.java @@ -34,6 +34,8 @@ import android.content.Intent; import android.content.pm.ApplicationInfo; import android.content.pm.IPackageMoveObserver; import android.content.pm.PackageManager; +import android.content.res.ObbInfo; +import android.content.res.ObbScanner; import android.os.Binder; import android.os.Environment; import android.os.FileUtils; @@ -580,7 +582,8 @@ public class StorageManager { try { final String canonicalPath = new File(rawPath).getCanonicalPath(); final int nonce = mObbActionListener.addListener(listener); - mStorageManager.mountObb(rawPath, canonicalPath, key, mObbActionListener, nonce); + mStorageManager.mountObb(rawPath, canonicalPath, key, mObbActionListener, nonce, + getObbInfo(canonicalPath)); return true; } catch (IOException e) { throw new IllegalArgumentException("Failed to resolve path: " + rawPath, e); @@ -589,6 +592,15 @@ public class StorageManager { } } + private ObbInfo getObbInfo(String canonicalPath) { + try { + final ObbInfo obbInfo = ObbScanner.getObbInfo(canonicalPath); + return obbInfo; + } catch (IOException e) { + throw new IllegalArgumentException("Couldn't get OBB info for " + canonicalPath, e); + } + } + /** * Unmount an Opaque Binary Blob (OBB) file asynchronously. If the * force flag is true, it will kill any application needed to diff --git a/core/tests/coretests/src/android/os/storage/StorageManagerBaseTest.java b/core/tests/coretests/src/android/os/storage/StorageManagerBaseTest.java index 56629d4c4494..16dcff5e0c8c 100644 --- a/core/tests/coretests/src/android/os/storage/StorageManagerBaseTest.java +++ b/core/tests/coretests/src/android/os/storage/StorageManagerBaseTest.java @@ -65,95 +65,6 @@ public class StorageManagerBaseTest extends InstrumentationTestCase { + "to ourselves and our posterity, do ordain and establish this Constitution\n" + "for the United States of America.\n\n"; - class MountingObbThread extends Thread { - boolean mStop = false; - volatile boolean mFileOpenOnObb = false; - private String mObbFilePath = null; - private String mPathToContentsFile = null; - private String mOfficialObbFilePath = null; - - /** - * Constructor - * - * @param obbFilePath path to the OBB image file - * @param pathToContentsFile path to a file on the mounted OBB volume to open after the OBB - * has been mounted - */ - public MountingObbThread (String obbFilePath, String pathToContentsFile) { - assertTrue("obbFilePath cannot be null!", obbFilePath != null); - mObbFilePath = obbFilePath; - assertTrue("path to contents file cannot be null!", pathToContentsFile != null); - mPathToContentsFile = pathToContentsFile; - } - - /** - * Runs the thread - * - * Mounts OBB_FILE_1, and tries to open a file on the mounted OBB (specified in the - * constructor). Once it's open, it waits until someone calls its doStop(), after which it - * closes the opened file. - */ - public void run() { - // the official OBB file path and the mount-request file path should be the same, but - // let's distinguish the two as they may make for some interesting tests later - mOfficialObbFilePath = mountObb(mObbFilePath); - assertEquals("Expected and actual OBB file paths differ!", mObbFilePath, - mOfficialObbFilePath); - - // open a file on OBB 1... - DataInputStream inputFile = openFileOnMountedObb(mOfficialObbFilePath, - mPathToContentsFile); - assertTrue("Failed to open file!", inputFile != null); - - synchronized (this) { - mFileOpenOnObb = true; - notifyAll(); - } - - while (!mStop) { - try { - Thread.sleep(WAIT_TIME_INCR); - } catch (InterruptedException e) { - // nothing special to be done for interruptions - } - } - try { - inputFile.close(); - } catch (IOException e) { - fail("Failed to close file on OBB due to error: " + e.toString()); - } - } - - /** - * Tells whether a file has yet been successfully opened on the OBB or not - * - * @return true if the specified file on the OBB was opened; false otherwise - */ - public boolean isFileOpenOnObb() { - return mFileOpenOnObb; - } - - /** - * Returns the official path of the OBB file that was mounted - * - * This is not the mount path, but the normalized path to the actual OBB file - * - * @return a {@link String} representation of the path to the OBB file that was mounted - */ - public String officialObbFilePath() { - return mOfficialObbFilePath; - } - - /** - * Requests the thread to stop running - * - * Closes the opened file and returns - */ - public void doStop() { - mStop = true; - } - } - public class ObbListener extends OnObbStateChangeListener { private String LOG_TAG = "StorageManagerBaseTest.ObbListener"; @@ -362,7 +273,8 @@ public class StorageManagerBaseTest extends InstrumentationTestCase { assertTrue("mountObb call failed", mSm.mountObb(obbFilePath, key, obbListener)); assertTrue("Failed to get OBB mount status change for file: " + obbFilePath, doWaitForObbStateChange(obbListener)); - assertEquals("OBB mount state not what was expected!", expectedState, obbListener.state()); + assertEquals("OBB mount state not what was expected!", expectedState, + obbListener.state()); if (OnObbStateChangeListener.MOUNTED == expectedState) { assertEquals(obbFilePath, obbListener.officialPath()); @@ -373,7 +285,8 @@ public class StorageManagerBaseTest extends InstrumentationTestCase { mSm.isObbMounted(obbListener.officialPath())); } - assertEquals("Mount state is not what was expected!", expectedState, obbListener.state()); + assertEquals("Mount state is not what was expected!", expectedState, + obbListener.state()); return obbListener.officialPath(); } diff --git a/core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java b/core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java index 5f8bd03ad50d..3ec297c27a2a 100644 --- a/core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java +++ b/core/tests/coretests/src/android/os/storage/StorageManagerIntegrationTest.java @@ -20,7 +20,6 @@ import android.os.ParcelFileDescriptor; import android.os.ProxyFileDescriptorCallback; import android.system.ErrnoException; import android.test.suitebuilder.annotation.LargeTest; -import android.util.Log; import com.android.frameworks.coretests.R; @@ -134,50 +133,6 @@ public class StorageManagerIntegrationTest extends StorageManagerBaseTest { } } - /** - * Tests that we can not force unmount when a file is currently open on the OBB. - */ - @LargeTest - public void testUnmount_DontForce() throws Exception { - final File file = createObbFile(OBB_FILE_1, R.raw.obb_file1); - String obbFilePath = file.getAbsolutePath(); - - MountingObbThread mountingThread = new MountingObbThread(obbFilePath, - OBB_FILE_1_CONTENTS_1); - - try { - mountingThread.start(); - - long waitTime = 0; - while (!mountingThread.isFileOpenOnObb()) { - synchronized (mountingThread) { - Log.i(LOG_TAG, "Waiting for file to be opened on OBB..."); - mountingThread.wait(WAIT_TIME_INCR); - waitTime += WAIT_TIME_INCR; - if (waitTime > MAX_WAIT_TIME) { - fail("Timed out waiting for file file to be opened on OBB!"); - } - } - } - - unmountObb(obbFilePath, DONT_FORCE); - - // verify still mounted - assertTrue("mounted path should not be null!", obbFilePath != null); - assertTrue("mounted path should still be mounted!", mSm.isObbMounted(obbFilePath)); - - // close the opened file - mountingThread.doStop(); - - // try unmounting again (should succeed this time) - unmountObb(obbFilePath, DONT_FORCE); - assertFalse("mounted path should no longer be mounted!", - mSm.isObbMounted(obbFilePath)); - } catch (InterruptedException e) { - fail("Timed out waiting for file on OBB to be opened..."); - } - } - /** * Tests mounting a single OBB that isn't signed. */ @@ -185,7 +140,12 @@ public class StorageManagerIntegrationTest extends StorageManagerBaseTest { public void testMountUnsignedObb() throws Exception { final File file = createObbFile(OBB_FILE_2_UNSIGNED, R.raw.obb_file2_nosign); String filePath = file.getAbsolutePath(); - mountObb(filePath, OBB_FILE_2_UNSIGNED, OnObbStateChangeListener.ERROR_INTERNAL); + try { + mountObb(filePath, OBB_FILE_2_UNSIGNED, OnObbStateChangeListener.ERROR_INTERNAL); + fail("mountObb should've failed with an exception"); + } catch (IllegalArgumentException e) { + // Expected + } } /** diff --git a/libs/storage/Android.bp b/libs/storage/Android.bp index 911bd1d25393..c19933e39c96 100644 --- a/libs/storage/Android.bp +++ b/libs/storage/Android.bp @@ -6,6 +6,7 @@ cc_library_static { "IMountShutdownObserver.cpp", "IObbActionListener.cpp", "IMountService.cpp", + "ObbInfo.cpp", ], export_include_dirs: ["include"], diff --git a/libs/storage/IMountService.cpp b/libs/storage/IMountService.cpp index fa3d8bd0930f..fd6e6e932ebc 100644 --- a/libs/storage/IMountService.cpp +++ b/libs/storage/IMountService.cpp @@ -443,7 +443,7 @@ public: } void mountObb(const String16& rawPath, const String16& canonicalPath, const String16& key, - const sp& token, int32_t nonce) + const sp& token, int32_t nonce, const sp& obbInfo) { Parcel data, reply; data.writeInterfaceToken(IMountService::getInterfaceDescriptor()); @@ -452,6 +452,7 @@ public: data.writeString16(key); data.writeStrongBinder(IInterface::asBinder(token)); data.writeInt32(nonce); + obbInfo->writeToParcel(&data); if (remote()->transact(TRANSACTION_mountObb, data, &reply) != NO_ERROR) { ALOGD("mountObb could not contact remote\n"); return; diff --git a/libs/storage/ObbInfo.cpp b/libs/storage/ObbInfo.cpp new file mode 100644 index 000000000000..1bb6b3a89b86 --- /dev/null +++ b/libs/storage/ObbInfo.cpp @@ -0,0 +1,47 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include +#include + +namespace android { + +ObbInfo::ObbInfo(const String16 fileName, const String16 packageName, int32_t version, + int32_t flags, size_t saltSize, const uint8_t* salt) : mFileName(fileName), + mPackageName(packageName), mVersion(version), mFlags(flags), mSaltSize(saltSize), + mSalt(salt) {} + +ObbInfo::~ObbInfo() {} + +status_t ObbInfo::readFromParcel(const Parcel*) { + return INVALID_OPERATION; +} + +status_t ObbInfo::writeToParcel(Parcel* p) const { + // Parcel write code must be kept in sync with + // frameworks/base/core/java/android/content/res/ObbInfo.java + p->writeString16(mFileName); + p->writeString16(mPackageName); + p->writeInt32(mVersion); + p->writeInt32(mFlags); + p->writeByteArray(mSaltSize, mSalt); + return OK; +} + +}; // namespace android \ No newline at end of file diff --git a/libs/storage/include/storage/IMountService.h b/libs/storage/include/storage/IMountService.h index c3d34d84958b..2463e023efc1 100644 --- a/libs/storage/include/storage/IMountService.h +++ b/libs/storage/include/storage/IMountService.h @@ -20,6 +20,7 @@ #include #include #include +#include #include @@ -64,7 +65,7 @@ public: virtual void finishMediaUpdate() = 0; virtual void mountObb(const String16& rawPath, const String16& canonicalPath, const String16& key, const sp& token, - const int32_t nonce) = 0; + const int32_t nonce, const sp& obbInfo) = 0; virtual void unmountObb(const String16& filename, const bool force, const sp& token, const int32_t nonce) = 0; virtual bool isObbMounted(const String16& filename) = 0; diff --git a/libs/storage/include/storage/ObbInfo.h b/libs/storage/include/storage/ObbInfo.h new file mode 100644 index 000000000000..e4cc353d64c7 --- /dev/null +++ b/libs/storage/include/storage/ObbInfo.h @@ -0,0 +1,48 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_OBBINFO_H +#define ANDROID_OBBINFO_H + +#include +#include +#include +#include + +namespace android { + +class ObbInfo : public Parcelable, public virtual RefBase { + +public: + ObbInfo(const String16 fileName, const String16 packageName, int32_t version, + int32_t flags, size_t saltSize, const uint8_t* salt); + ~ObbInfo(); + + status_t writeToParcel(Parcel* parcel) const override; + status_t readFromParcel(const Parcel* parcel) override; + +private: + const String16 mFileName; + const String16 mPackageName; + int32_t mVersion; + int32_t mFlags; + size_t mSaltSize; + const uint8_t* mSalt; +}; + +}; // namespace android + +#endif // ANDROID_OBBINFO_H \ No newline at end of file diff --git a/native/android/storage_manager.cpp b/native/android/storage_manager.cpp index bf15b8d075e7..22725254fef6 100644 --- a/native/android/storage_manager.cpp +++ b/native/android/storage_manager.cpp @@ -18,7 +18,9 @@ #include #include +#include +#include #include #include #include @@ -29,7 +31,6 @@ #include #include - using namespace android; struct ObbActionListener : public BnObbActionListener { @@ -79,6 +80,20 @@ protected: return cb; } + ObbInfo* getObbInfo(char* canonicalPath) { + sp obbFile = new ObbFile(); + if (!obbFile->readFrom(canonicalPath)) { + return nullptr; + } + + String16 fileName(obbFile->getFileName()); + String16 packageName(obbFile->getPackageName()); + size_t length; + const unsigned char* salt = obbFile->getSalt(&length); + return new ObbInfo(fileName, packageName, + obbFile->getVersion(), obbFile->getFlags(), length, salt); + } + public: AStorageManager() { @@ -134,11 +149,18 @@ public: return; } + sp obbInfo = getObbInfo(canonicalPath); + if (obbInfo == nullptr) { + ALOGE("Couldn't get obb info for %s: %s", canonicalPath, strerror(errno)); + return; + } + ObbCallback* cb = registerObbCallback(func, data); String16 rawPath16(rawPath); String16 canonicalPath16(canonicalPath); String16 key16(key); - mMountService->mountObb(rawPath16, canonicalPath16, key16, mObbActionListener, cb->nonce); + mMountService->mountObb(rawPath16, canonicalPath16, key16, mObbActionListener, + cb->nonce, obbInfo); } void unmountObb(const char* filename, const bool force, AStorageManager_obbCallbackFunc func, void* data) { diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java index 1746b857cf5c..d505a77c9192 100644 --- a/services/core/java/com/android/server/StorageManagerService.java +++ b/services/core/java/com/android/server/StorageManagerService.java @@ -44,11 +44,9 @@ import android.app.KeyguardManager; import android.app.admin.SecurityLog; import android.app.usage.StorageStatsManager; import android.content.BroadcastReceiver; -import android.content.ComponentName; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; -import android.content.ServiceConnection; import android.content.pm.ApplicationInfo; import android.content.pm.IPackageMoveObserver; import android.content.pm.PackageManager; @@ -115,7 +113,6 @@ import android.util.Xml; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; -import com.android.internal.app.IMediaContainerService; import com.android.internal.os.AppFuseMount; import com.android.internal.os.BackgroundThread; import com.android.internal.os.FuseUnavailableMountException; @@ -544,37 +541,7 @@ class StorageManagerService extends IStorageManager.Stub // OBB action handler messages private static final int OBB_RUN_ACTION = 1; - private static final int OBB_MCS_BOUND = 2; - private static final int OBB_MCS_UNBIND = 3; - private static final int OBB_MCS_RECONNECT = 4; - private static final int OBB_FLUSH_MOUNT_STATE = 5; - - /* - * Default Container Service information - */ - static final ComponentName DEFAULT_CONTAINER_COMPONENT = new ComponentName( - "com.android.defcontainer", "com.android.defcontainer.DefaultContainerService"); - - final private DefaultContainerConnection mDefContainerConn = new DefaultContainerConnection(); - - class DefaultContainerConnection implements ServiceConnection { - @Override - public void onServiceConnected(ComponentName name, IBinder service) { - if (DEBUG_OBB) - Slog.i(TAG, "onServiceConnected"); - IMediaContainerService imcs = IMediaContainerService.Stub.asInterface(service); - mObbActionHandler.sendMessage(mObbActionHandler.obtainMessage(OBB_MCS_BOUND, imcs)); - } - - @Override - public void onServiceDisconnected(ComponentName name) { - if (DEBUG_OBB) - Slog.i(TAG, "onServiceDisconnected"); - } - } - - // Used in the ObbActionHandler - private IMediaContainerService mContainerService = null; + private static final int OBB_FLUSH_MOUNT_STATE = 2; // Last fstrim operation tracking private static final String LAST_FSTRIM_FILE = "last-fstrim"; @@ -2305,16 +2272,17 @@ class StorageManagerService extends IStorageManager.Stub } @Override - public void mountObb( - String rawPath, String canonicalPath, String key, IObbActionListener token, int nonce) { + public void mountObb(String rawPath, String canonicalPath, String key, + IObbActionListener token, int nonce, ObbInfo obbInfo) { Preconditions.checkNotNull(rawPath, "rawPath cannot be null"); Preconditions.checkNotNull(canonicalPath, "canonicalPath cannot be null"); Preconditions.checkNotNull(token, "token cannot be null"); + Preconditions.checkNotNull(obbInfo, "obbIfno cannot be null"); final int callingUid = Binder.getCallingUid(); final ObbState obbState = new ObbState(rawPath, canonicalPath, callingUid, token, nonce, null); - final ObbAction action = new MountObbAction(obbState, key, callingUid); + final ObbAction action = new MountObbAction(obbState, key, callingUid, obbInfo); mObbActionHandler.sendMessage(mObbActionHandler.obtainMessage(OBB_RUN_ACTION, action)); if (DEBUG_OBB) @@ -3217,8 +3185,6 @@ class StorageManagerService extends IStorageManager.Stub } private class ObbActionHandler extends Handler { - private boolean mBound = false; - private final List mActions = new LinkedList(); ObbActionHandler(Looper l) { super(l); @@ -3233,83 +3199,7 @@ class StorageManagerService extends IStorageManager.Stub if (DEBUG_OBB) Slog.i(TAG, "OBB_RUN_ACTION: " + action.toString()); - // If a bind was already initiated we don't really - // need to do anything. The pending install - // will be processed later on. - if (!mBound) { - // If this is the only one pending we might - // have to bind to the service again. - if (!connectToService()) { - action.notifyObbStateChange(new ObbException(ERROR_INTERNAL, - "Failed to bind to media container service")); - return; - } - } - - mActions.add(action); - break; - } - case OBB_MCS_BOUND: { - if (DEBUG_OBB) - Slog.i(TAG, "OBB_MCS_BOUND"); - if (msg.obj != null) { - mContainerService = (IMediaContainerService) msg.obj; - } - if (mContainerService == null) { - // Something seriously wrong. Bail out - for (ObbAction action : mActions) { - // Indicate service bind error - action.notifyObbStateChange(new ObbException(ERROR_INTERNAL, - "Failed to bind to media container service")); - } - mActions.clear(); - } else if (mActions.size() > 0) { - final ObbAction action = mActions.get(0); - if (action != null) { - action.execute(this); - } - } else { - // Should never happen ideally. - Slog.w(TAG, "Empty queue"); - } - break; - } - case OBB_MCS_RECONNECT: { - if (DEBUG_OBB) - Slog.i(TAG, "OBB_MCS_RECONNECT"); - if (mActions.size() > 0) { - if (mBound) { - disconnectService(); - } - if (!connectToService()) { - for (ObbAction action : mActions) { - // Indicate service bind error - action.notifyObbStateChange(new ObbException(ERROR_INTERNAL, - "Failed to bind to media container service")); - } - mActions.clear(); - } - } - break; - } - case OBB_MCS_UNBIND: { - if (DEBUG_OBB) - Slog.i(TAG, "OBB_MCS_UNBIND"); - - // Delete pending install - if (mActions.size() > 0) { - mActions.remove(0); - } - if (mActions.size() == 0) { - if (mBound) { - disconnectService(); - } - } else { - // There are more pending requests in queue. - // Just post MCS_BOUND message to trigger processing - // of next pending install. - mObbActionHandler.sendEmptyMessage(OBB_MCS_BOUND); - } + action.execute(this); break; } case OBB_FLUSH_MOUNT_STATE: { @@ -3354,25 +3244,6 @@ class StorageManagerService extends IStorageManager.Stub } } } - - private boolean connectToService() { - if (DEBUG_OBB) - Slog.i(TAG, "Trying to bind to DefaultContainerService"); - - Intent service = new Intent().setComponent(DEFAULT_CONTAINER_COMPONENT); - if (mContext.bindServiceAsUser(service, mDefContainerConn, Context.BIND_AUTO_CREATE, - UserHandle.SYSTEM)) { - mBound = true; - return true; - } - return false; - } - - private void disconnectService() { - mContainerService = null; - mBound = false; - mContext.unbindService(mDefContainerConn); - } } private static class ObbException extends Exception { @@ -3390,8 +3261,6 @@ class StorageManagerService extends IStorageManager.Stub } abstract class ObbAction { - private static final int MAX_RETRIES = 3; - private int mRetries; ObbState mObbState; @@ -3403,40 +3272,14 @@ class StorageManagerService extends IStorageManager.Stub try { if (DEBUG_OBB) Slog.i(TAG, "Starting to execute action: " + toString()); - mRetries++; - if (mRetries > MAX_RETRIES) { - mObbActionHandler.sendEmptyMessage(OBB_MCS_UNBIND); - notifyObbStateChange(new ObbException(ERROR_INTERNAL, - "Failed to bind to media container service")); - } else { - handleExecute(); - if (DEBUG_OBB) - Slog.i(TAG, "Posting install MCS_UNBIND"); - mObbActionHandler.sendEmptyMessage(OBB_MCS_UNBIND); - } + handleExecute(); } catch (ObbException e) { notifyObbStateChange(e); - mObbActionHandler.sendEmptyMessage(OBB_MCS_UNBIND); } } abstract void handleExecute() throws ObbException; - protected ObbInfo getObbInfo() throws ObbException { - final ObbInfo obbInfo; - try { - obbInfo = mContainerService.getObbInfo(mObbState.canonicalPath); - } catch (Exception e) { - throw new ObbException(ERROR_PERMISSION_DENIED, e); - } - if (obbInfo != null) { - return obbInfo; - } else { - throw new ObbException(ERROR_INTERNAL, - "Missing OBB info for: " + mObbState.canonicalPath); - } - } - protected void notifyObbStateChange(ObbException e) { Slog.w(TAG, e); notifyObbStateChange(e.status); @@ -3458,22 +3301,22 @@ class StorageManagerService extends IStorageManager.Stub class MountObbAction extends ObbAction { private final String mKey; private final int mCallingUid; + private ObbInfo mObbInfo; - MountObbAction(ObbState obbState, String key, int callingUid) { + MountObbAction(ObbState obbState, String key, int callingUid, ObbInfo obbInfo) { super(obbState); mKey = key; mCallingUid = callingUid; + mObbInfo = obbInfo; } @Override public void handleExecute() throws ObbException { warnOnNotMounted(); - final ObbInfo obbInfo = getObbInfo(); - - if (!isUidOwnerOfPackageOrSystem(obbInfo.packageName, mCallingUid)) { + if (!isUidOwnerOfPackageOrSystem(mObbInfo.packageName, mCallingUid)) { throw new ObbException(ERROR_PERMISSION_DENIED, "Denied attempt to mount OBB " - + obbInfo.filename + " which is owned by " + obbInfo.packageName); + + mObbInfo.filename + " which is owned by " + mObbInfo.packageName); } final boolean isMounted; @@ -3482,7 +3325,7 @@ class StorageManagerService extends IStorageManager.Stub } if (isMounted) { throw new ObbException(ERROR_ALREADY_MOUNTED, - "Attempt to mount OBB which is already mounted: " + obbInfo.filename); + "Attempt to mount OBB which is already mounted: " + mObbInfo.filename); } final String hashedKey; @@ -3494,7 +3337,7 @@ class StorageManagerService extends IStorageManager.Stub try { SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1"); - KeySpec ks = new PBEKeySpec(mKey.toCharArray(), obbInfo.salt, + KeySpec ks = new PBEKeySpec(mKey.toCharArray(), mObbInfo.salt, PBKDF2_HASH_ROUNDS, CRYPTO_ALGORITHM_KEY_SIZE); SecretKey key = factory.generateSecret(ks); BigInteger bi = new BigInteger(key.getEncoded()); diff --git a/services/tests/servicestests/src/com/android/server/MountServiceTests.java b/services/tests/servicestests/src/com/android/server/MountServiceTests.java index ecfe0db103fe..b1b31744c88b 100644 --- a/services/tests/servicestests/src/com/android/server/MountServiceTests.java +++ b/services/tests/servicestests/src/com/android/server/MountServiceTests.java @@ -220,7 +220,12 @@ public class MountServiceTests extends AndroidTestCase { final File outFile = getFilePath("test1_nosig.obb"); - mountObb(sm, R.raw.test1_nosig, outFile, OnObbStateChangeListener.ERROR_INTERNAL); + try { + mountObb(sm, R.raw.test1_nosig, outFile, OnObbStateChangeListener.ERROR_INTERNAL); + fail("mountObb should've failed with an exception"); + } catch (IllegalArgumentException e) { + // Expected + } assertFalse("OBB should not be mounted", sm.isObbMounted(outFile.getPath())); -- cgit v1.2.3-59-g8ed1b