summaryrefslogtreecommitdiffstats
path: root/packages/ExternalStorageProvider
diff options
context:
space:
mode:
authorJeff Sharkey <jsharkey@android.com>2014-11-26 13:38:26 -0800
committerJeff Sharkey <jsharkey@android.com>2014-12-01 09:38:49 -0800
commit0cce5355b45d835f95a8918b8b803fd977d374e4 (patch)
treed42b69e266761ba6caed601f5d710df9d9739bb5 /packages/ExternalStorageProvider
parent48e17629b0b6c89cb77342c0364a1cf3a0b2a0fb (diff)
downloadframeworks_base-0cce5355b45d835f95a8918b8b803fd977d374e4.zip
frameworks_base-0cce5355b45d835f95a8918b8b803fd977d374e4.tar.gz
frameworks_base-0cce5355b45d835f95a8918b8b803fd977d374e4.tar.bz2
Sanitize display names, keep extensions intact.
When creating or renaming files on external storage, sanitize the requested display names to be valid FAT filenames. This also fixes a handful of directory traversal bugs. Also relax logic around generating display names to allow any extension which maps to the requested MIME type. Tests to verify. Bug: 18512473, 18504132 Change-Id: I89e632019ee145f53d9d9d2050932f8939a756af
Diffstat (limited to 'packages/ExternalStorageProvider')
-rw-r--r--packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java111
-rw-r--r--packages/ExternalStorageProvider/tests/Android.mk16
-rw-r--r--packages/ExternalStorageProvider/tests/AndroidManifest.xml13
-rw-r--r--packages/ExternalStorageProvider/tests/src/com/android/externalstorage/ExternalStorageProviderTest.java90
4 files changed, 191 insertions, 39 deletions
diff --git a/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java b/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
index 066acac..073d9c7 100644
--- a/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
+++ b/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
@@ -43,6 +43,7 @@ import android.util.Log;
import android.webkit.MimeTypeMap;
import com.android.internal.annotations.GuardedBy;
+import com.android.internal.annotations.VisibleForTesting;
import com.google.android.collect.Lists;
import com.google.android.collect.Maps;
@@ -53,6 +54,7 @@ import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.Map;
+import java.util.Objects;
public class ExternalStorageProvider extends DocumentsProvider {
private static final String TAG = "ExternalStorage";
@@ -313,27 +315,19 @@ public class ExternalStorageProvider extends DocumentsProvider {
@Override
public String createDocument(String docId, String mimeType, String displayName)
throws FileNotFoundException {
+ displayName = FileUtils.buildValidFatFilename(displayName);
+
final File parent = getFileForDocId(docId);
if (!parent.isDirectory()) {
throw new IllegalArgumentException("Parent document isn't a directory");
}
- File file;
+ final File file = buildUniqueFile(parent, mimeType, displayName);
if (Document.MIME_TYPE_DIR.equals(mimeType)) {
- file = new File(parent, displayName);
if (!file.mkdir()) {
throw new IllegalStateException("Failed to mkdir " + file);
}
} else {
- displayName = removeExtension(mimeType, displayName);
- file = new File(parent, addExtension(mimeType, displayName));
-
- // If conflicting file, try adding counter suffix
- int n = 0;
- while (file.exists() && n++ < 32) {
- file = new File(parent, addExtension(mimeType, displayName + " (" + n + ")"));
- }
-
try {
if (!file.createNewFile()) {
throw new IllegalStateException("Failed to touch " + file);
@@ -342,11 +336,78 @@ public class ExternalStorageProvider extends DocumentsProvider {
throw new IllegalStateException("Failed to touch " + file + ": " + e);
}
}
+
return getDocIdForFile(file);
}
+ private static File buildFile(File parent, String name, String ext) {
+ if (TextUtils.isEmpty(ext)) {
+ return new File(parent, name);
+ } else {
+ return new File(parent, name + "." + ext);
+ }
+ }
+
+ @VisibleForTesting
+ public static File buildUniqueFile(File parent, String mimeType, String displayName)
+ throws FileNotFoundException {
+ String name;
+ String ext;
+
+ if (Document.MIME_TYPE_DIR.equals(mimeType)) {
+ name = displayName;
+ ext = null;
+ } else {
+ String mimeTypeFromExt;
+
+ // Extract requested extension from display name
+ final int lastDot = displayName.lastIndexOf('.');
+ if (lastDot >= 0) {
+ name = displayName.substring(0, lastDot);
+ ext = displayName.substring(lastDot + 1);
+ mimeTypeFromExt = MimeTypeMap.getSingleton().getMimeTypeFromExtension(
+ ext.toLowerCase());
+ } else {
+ name = displayName;
+ ext = null;
+ mimeTypeFromExt = null;
+ }
+
+ if (mimeTypeFromExt == null) {
+ mimeTypeFromExt = "application/octet-stream";
+ }
+
+ final String extFromMimeType = MimeTypeMap.getSingleton().getExtensionFromMimeType(
+ mimeType);
+ if (Objects.equals(mimeType, mimeTypeFromExt) || Objects.equals(ext, extFromMimeType)) {
+ // Extension maps back to requested MIME type; allow it
+ } else {
+ // No match; insist that create file matches requested MIME
+ name = displayName;
+ ext = extFromMimeType;
+ }
+ }
+
+ File file = buildFile(parent, name, ext);
+
+ // If conflicting file, try adding counter suffix
+ int n = 0;
+ while (file.exists()) {
+ if (n++ >= 32) {
+ throw new FileNotFoundException("Failed to create unique file");
+ }
+ file = buildFile(parent, name + " (" + n + ")", ext);
+ }
+
+ return file;
+ }
+
@Override
public String renameDocument(String docId, String displayName) throws FileNotFoundException {
+ // Since this provider treats renames as generating a completely new
+ // docId, we're okay with letting the MIME type change.
+ displayName = FileUtils.buildValidFatFilename(displayName);
+
final File before = getFileForDocId(docId);
final File after = new File(before.getParentFile(), displayName);
if (after.exists()) {
@@ -482,34 +543,6 @@ public class ExternalStorageProvider extends DocumentsProvider {
return "application/octet-stream";
}
- /**
- * Remove file extension from name, but only if exact MIME type mapping
- * exists. This means we can reapply the extension later.
- */
- private static String removeExtension(String mimeType, String name) {
- final int lastDot = name.lastIndexOf('.');
- if (lastDot >= 0) {
- final String extension = name.substring(lastDot + 1).toLowerCase();
- final String nameMime = MimeTypeMap.getSingleton().getMimeTypeFromExtension(extension);
- if (mimeType.equals(nameMime)) {
- return name.substring(0, lastDot);
- }
- }
- return name;
- }
-
- /**
- * Add file extension to name, but only if exact MIME type mapping exists.
- */
- private static String addExtension(String mimeType, String name) {
- final String extension = MimeTypeMap.getSingleton()
- .getExtensionFromMimeType(mimeType);
- if (extension != null) {
- return name + "." + extension;
- }
- return name;
- }
-
private void startObserving(File file, Uri notifyUri) {
synchronized (mObservers) {
DirectoryObserver observer = mObservers.get(file);
diff --git a/packages/ExternalStorageProvider/tests/Android.mk b/packages/ExternalStorageProvider/tests/Android.mk
new file mode 100644
index 0000000..830731a
--- /dev/null
+++ b/packages/ExternalStorageProvider/tests/Android.mk
@@ -0,0 +1,16 @@
+
+LOCAL_PATH := $(call my-dir)
+include $(CLEAR_VARS)
+
+LOCAL_MODULE_TAGS := tests
+
+LOCAL_SRC_FILES := $(call all-java-files-under, src)
+
+LOCAL_JAVA_LIBRARIES := android.test.runner
+
+LOCAL_PACKAGE_NAME := ExternalStorageProviderTests
+LOCAL_INSTRUMENTATION_FOR := ExternalStorageProvider
+
+LOCAL_CERTIFICATE := platform
+
+include $(BUILD_PACKAGE)
diff --git a/packages/ExternalStorageProvider/tests/AndroidManifest.xml b/packages/ExternalStorageProvider/tests/AndroidManifest.xml
new file mode 100644
index 0000000..ffcd499
--- /dev/null
+++ b/packages/ExternalStorageProvider/tests/AndroidManifest.xml
@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="utf-8"?>
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+ package="com.android.externalstorage.tests">
+
+ <application>
+ <uses-library android:name="android.test.runner" />
+ </application>
+
+ <instrumentation android:name="android.test.InstrumentationTestRunner"
+ android:targetPackage="com.android.externalstorage"
+ android:label="Tests for ExternalStorageProvider" />
+
+</manifest>
diff --git a/packages/ExternalStorageProvider/tests/src/com/android/externalstorage/ExternalStorageProviderTest.java b/packages/ExternalStorageProvider/tests/src/com/android/externalstorage/ExternalStorageProviderTest.java
new file mode 100644
index 0000000..f980b60
--- /dev/null
+++ b/packages/ExternalStorageProvider/tests/src/com/android/externalstorage/ExternalStorageProviderTest.java
@@ -0,0 +1,90 @@
+/*
+ * Copyright (C) 2013 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.
+ */
+
+package com.android.externalstorage;
+
+import static com.android.externalstorage.ExternalStorageProvider.buildUniqueFile;
+
+import android.os.FileUtils;
+import android.provider.DocumentsContract.Document;
+import android.test.AndroidTestCase;
+import android.test.suitebuilder.annotation.MediumTest;
+
+import java.io.File;
+
+@MediumTest
+public class ExternalStorageProviderTest extends AndroidTestCase {
+
+ private File mTarget;
+
+ @Override
+ protected void setUp() throws Exception {
+ super.setUp();
+ mTarget = getContext().getFilesDir();
+ FileUtils.deleteContents(mTarget);
+ }
+
+ @Override
+ protected void tearDown() throws Exception {
+ super.tearDown();
+ FileUtils.deleteContents(mTarget);
+ }
+
+ public void testBuildUniqueFile_normal() throws Exception {
+ assertNameEquals("test.jpg", buildUniqueFile(mTarget, "image/jpeg", "test"));
+ assertNameEquals("test.jpg", buildUniqueFile(mTarget, "image/jpeg", "test.jpg"));
+ assertNameEquals("test.jpeg", buildUniqueFile(mTarget, "image/jpeg", "test.jpeg"));
+ assertNameEquals("TEst.JPeg", buildUniqueFile(mTarget, "image/jpeg", "TEst.JPeg"));
+ assertNameEquals("test.png.jpg", buildUniqueFile(mTarget, "image/jpeg", "test.png.jpg"));
+ assertNameEquals("test.png.jpg", buildUniqueFile(mTarget, "image/jpeg", "test.png"));
+
+ assertNameEquals("test.flac", buildUniqueFile(mTarget, "audio/flac", "test"));
+ assertNameEquals("test.flac", buildUniqueFile(mTarget, "audio/flac", "test.flac"));
+ assertNameEquals("test.flac", buildUniqueFile(mTarget, "application/x-flac", "test"));
+ assertNameEquals("test.flac", buildUniqueFile(mTarget, "application/x-flac", "test.flac"));
+ }
+
+ public void testBuildUniqueFile_unknown() throws Exception {
+ assertNameEquals("test", buildUniqueFile(mTarget, "application/octet-stream", "test"));
+ assertNameEquals("test.jpg", buildUniqueFile(mTarget, "application/octet-stream", "test.jpg"));
+ assertNameEquals(".test", buildUniqueFile(mTarget, "application/octet-stream", ".test"));
+
+ assertNameEquals("test", buildUniqueFile(mTarget, "lolz/lolz", "test"));
+ assertNameEquals("test.lolz", buildUniqueFile(mTarget, "lolz/lolz", "test.lolz"));
+ }
+
+ public void testBuildUniqueFile_dir() throws Exception {
+ assertNameEquals("test", buildUniqueFile(mTarget, Document.MIME_TYPE_DIR, "test"));
+ new File(mTarget, "test").mkdir();
+ assertNameEquals("test (1)", buildUniqueFile(mTarget, Document.MIME_TYPE_DIR, "test"));
+
+ assertNameEquals("test.jpg", buildUniqueFile(mTarget, Document.MIME_TYPE_DIR, "test.jpg"));
+ new File(mTarget, "test.jpg").mkdir();
+ assertNameEquals("test.jpg (1)", buildUniqueFile(mTarget, Document.MIME_TYPE_DIR, "test.jpg"));
+ }
+
+ public void testBuildUniqueFile_increment() throws Exception {
+ assertNameEquals("test.jpg", buildUniqueFile(mTarget, "image/jpeg", "test.jpg"));
+ new File(mTarget, "test.jpg").createNewFile();
+ assertNameEquals("test (1).jpg", buildUniqueFile(mTarget, "image/jpeg", "test.jpg"));
+ new File(mTarget, "test (1).jpg").createNewFile();
+ assertNameEquals("test (2).jpg", buildUniqueFile(mTarget, "image/jpeg", "test.jpg"));
+ }
+
+ private static void assertNameEquals(String expected, File actual) {
+ assertEquals(expected, actual.getName());
+ }
+}