diff options
6 files changed, 310 insertions, 45 deletions
diff --git a/core/java/android/os/FileUtils.java b/core/java/android/os/FileUtils.java index fe47f5b..0a724a1 100644 --- a/core/java/android/os/FileUtils.java +++ b/core/java/android/os/FileUtils.java @@ -17,9 +17,8 @@ package android.os; import android.system.ErrnoException; -import android.text.TextUtils; import android.system.Os; -import android.system.OsConstants; +import android.text.TextUtils; import android.util.Log; import android.util.Slog; @@ -403,20 +402,89 @@ public class FileUtils { return success; } + private static boolean isValidExtFilenameChar(char c) { + switch (c) { + case '\0': + case '/': + return false; + default: + return true; + } + } + /** - * Assert that given filename is valid on ext4. + * Check if given filename is valid for an ext4 filesystem. */ public static boolean isValidExtFilename(String name) { + return (name != null) && name.equals(buildValidExtFilename(name)); + } + + /** + * Mutate the given filename to make it valid for an ext4 filesystem, + * replacing any invalid characters with "_". + */ + public static String buildValidExtFilename(String name) { if (TextUtils.isEmpty(name) || ".".equals(name) || "..".equals(name)) { - return false; + return "(invalid)"; } + final StringBuilder res = new StringBuilder(name.length()); for (int i = 0; i < name.length(); i++) { final char c = name.charAt(i); - if (c == '\0' || c == '/') { + if (isValidExtFilenameChar(c)) { + res.append(c); + } else { + res.append('_'); + } + } + return res.toString(); + } + + private static boolean isValidFatFilenameChar(char c) { + if ((0x00 <= c && c <= 0x1f)) { + return false; + } + switch (c) { + case '"': + case '*': + case '/': + case ':': + case '<': + case '>': + case '?': + case '\\': + case '|': + case 0x7F: return false; + default: + return true; + } + } + + /** + * Check if given filename is valid for a FAT filesystem. + */ + public static boolean isValidFatFilename(String name) { + return (name != null) && name.equals(buildValidFatFilename(name)); + } + + /** + * Mutate the given filename to make it valid for a FAT filesystem, + * replacing any invalid characters with "_". + */ + public static String buildValidFatFilename(String name) { + if (TextUtils.isEmpty(name) || ".".equals(name) || "..".equals(name)) { + return "(invalid)"; + } + final StringBuilder res = new StringBuilder(name.length()); + for (int i = 0; i < name.length(); i++) { + final char c = name.charAt(i); + if (isValidFatFilenameChar(c)) { + res.append(c); + } else { + res.append('_'); } } - return true; + return res.toString(); } public static String rewriteAfterRename(File beforeDir, File afterDir, String path) { diff --git a/core/tests/coretests/src/android/os/FileUtilsTest.java b/core/tests/coretests/src/android/os/FileUtilsTest.java index 93e68eb..5c9e813 100644 --- a/core/tests/coretests/src/android/os/FileUtilsTest.java +++ b/core/tests/coretests/src/android/os/FileUtilsTest.java @@ -180,6 +180,51 @@ public class FileUtilsTest extends AndroidTestCase { assertDirContents("file1", "file2"); } + public void testValidExtFilename() throws Exception { + assertTrue(FileUtils.isValidExtFilename("a")); + assertTrue(FileUtils.isValidExtFilename("foo.bar")); + assertTrue(FileUtils.isValidExtFilename("foo bar.baz")); + assertTrue(FileUtils.isValidExtFilename("foo.bar.baz")); + assertTrue(FileUtils.isValidExtFilename(".bar")); + assertTrue(FileUtils.isValidExtFilename("foo~!@#$%^&*()_[]{}+bar")); + + assertFalse(FileUtils.isValidExtFilename(null)); + assertFalse(FileUtils.isValidExtFilename(".")); + assertFalse(FileUtils.isValidExtFilename("../foo")); + assertFalse(FileUtils.isValidExtFilename("/foo")); + + assertEquals(".._foo", FileUtils.buildValidExtFilename("../foo")); + assertEquals("_foo", FileUtils.buildValidExtFilename("/foo")); + assertEquals("foo_bar", FileUtils.buildValidExtFilename("foo\0bar")); + assertEquals(".foo", FileUtils.buildValidExtFilename(".foo")); + assertEquals("foo.bar", FileUtils.buildValidExtFilename("foo.bar")); + } + + public void testValidFatFilename() throws Exception { + assertTrue(FileUtils.isValidFatFilename("a")); + assertTrue(FileUtils.isValidFatFilename("foo bar.baz")); + assertTrue(FileUtils.isValidFatFilename("foo.bar.baz")); + assertTrue(FileUtils.isValidFatFilename(".bar")); + assertTrue(FileUtils.isValidFatFilename("foo.bar")); + assertTrue(FileUtils.isValidFatFilename("foo bar")); + assertTrue(FileUtils.isValidFatFilename("foo+bar")); + assertTrue(FileUtils.isValidFatFilename("foo,bar")); + + assertFalse(FileUtils.isValidFatFilename("foo*bar")); + assertFalse(FileUtils.isValidFatFilename("foo?bar")); + assertFalse(FileUtils.isValidFatFilename("foo<bar")); + assertFalse(FileUtils.isValidFatFilename(null)); + assertFalse(FileUtils.isValidFatFilename(".")); + assertFalse(FileUtils.isValidFatFilename("../foo")); + assertFalse(FileUtils.isValidFatFilename("/foo")); + + assertEquals(".._foo", FileUtils.buildValidFatFilename("../foo")); + assertEquals("_foo", FileUtils.buildValidFatFilename("/foo")); + assertEquals(".foo", FileUtils.buildValidFatFilename(".foo")); + assertEquals("foo.bar", FileUtils.buildValidFatFilename("foo.bar")); + assertEquals("foo_bar__baz", FileUtils.buildValidFatFilename("foo?bar**baz")); + } + private void touch(String name, long age) throws Exception { final File file = new File(mDir, name); file.createNewFile(); 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()); + } +} |