[RF1] Remove files that don't match the expected format, step 1

This implements the skeleton implementation, and enables a
fallback implementation in the case the file is coming from the
dictionary provider.
- A better scheme should be used for provider-supplied dicts.
- This does not implement the solution for device-generated
    dicts yet. This will come in a future change.
- This does not implement the checking process on the native
    side yet. This will come in a future change.

Bug: 11281748
Change-Id: Ifa6e237d19dfbffe503e3674915480ea867b0ddf
main
Jean Chalard 2013-12-04 12:47:05 +09:00
parent 294fe541c8
commit 5e80e699c4
6 changed files with 102 additions and 7 deletions

View File

@ -52,4 +52,12 @@ public final class AssetFileAddress {
if (!f.isFile()) return null; if (!f.isFile()) return null;
return new AssetFileAddress(filename, offset, length); return new AssetFileAddress(filename, offset, length);
} }
public boolean pointsToPhysicalFile() {
return 0 == mOffset;
}
public void deleteUnderlyingFile() {
new File(mFilename).delete();
}
} }

View File

@ -121,6 +121,7 @@ public final class BinaryDictionary extends Dictionary {
String[] attributeKeyStringArray, String[] attributeValueStringArray); String[] attributeKeyStringArray, String[] attributeValueStringArray);
private static native long openNative(String sourceDir, long dictOffset, long dictSize, private static native long openNative(String sourceDir, long dictOffset, long dictSize,
boolean isUpdatable); boolean isUpdatable);
private static native boolean hasValidContentsNative(long dict);
private static native void flushNative(long dict, String filePath); private static native void flushNative(long dict, String filePath);
private static native boolean needsToRunGCNative(long dict, boolean mindsBlockByGC); private static native boolean needsToRunGCNative(long dict, boolean mindsBlockByGC);
private static native void flushWithGCNative(long dict, String filePath); private static native void flushWithGCNative(long dict, String filePath);
@ -242,6 +243,10 @@ public final class BinaryDictionary extends Dictionary {
return mNativeDict != 0; return mNativeDict != 0;
} }
public boolean hasValidContents() {
return hasValidContentsNative(mNativeDict);
}
public int getFormatVersion() { public int getFormatVersion() {
return getFormatVersionNative(mNativeDict); return getFormatVersionNative(mNativeDict);
} }
@ -376,7 +381,7 @@ public final class BinaryDictionary extends Dictionary {
private void reopen() { private void reopen() {
close(); close();
final File dictFile = new File(mDictFilePath); final File dictFile = new File(mDictFilePath);
// WARNING: Because we pass 0 as the offstet and file.length() as the length, this can // WARNING: Because we pass 0 as the offset and file.length() as the length, this can
// only be called for actual files. Right now it's only called by the flush() family of // only be called for actual files. Right now it's only called by the flush() family of
// functions, which require an updatable dictionary, so it's okay. But beware. // functions, which require an updatable dictionary, so it's okay. But beware.
loadDictionary(dictFile.getAbsolutePath(), 0 /* startOffset */, loadDictionary(dictFile.getAbsolutePath(), 0 /* startOffset */,

View File

@ -98,7 +98,7 @@ public final class BinaryDictionaryFileDumper {
* This creates a URI builder able to build a URI pointing to the dictionary * This creates a URI builder able to build a URI pointing to the dictionary
* pack content provider for a specific dictionary id. * pack content provider for a specific dictionary id.
*/ */
private static Uri.Builder getProviderUriBuilder(final String path) { public static Uri.Builder getProviderUriBuilder(final String path) {
return new Uri.Builder().scheme(ContentResolver.SCHEME_CONTENT) return new Uri.Builder().scheme(ContentResolver.SCHEME_CONTENT)
.authority(DictionaryPackConstants.AUTHORITY).appendPath(path); .authority(DictionaryPackConstants.AUTHORITY).appendPath(path);
} }
@ -339,15 +339,25 @@ public final class BinaryDictionaryFileDumper {
Log.e(TAG, "Could not copy a word list. Will not be able to use it."); Log.e(TAG, "Could not copy a word list. Will not be able to use it.");
// If we can't copy it we should warn the dictionary provider so that it can mark it // If we can't copy it we should warn the dictionary provider so that it can mark it
// as invalid. // as invalid.
wordListUriBuilder.appendQueryParameter(QUERY_PARAMETER_DELETE_RESULT, reportBrokenFileToDictionaryProvider(providerClient, clientId, wordlistId);
QUERY_PARAMETER_FAILURE); }
public static boolean reportBrokenFileToDictionaryProvider(
final ContentProviderClient providerClient, final String clientId,
final String wordlistId) {
try { try {
final Uri.Builder wordListUriBuilder = getContentUriBuilderForType(clientId,
providerClient, QUERY_PATH_DATAFILE, wordlistId /* extraPath */);
wordListUriBuilder.appendQueryParameter(QUERY_PARAMETER_DELETE_RESULT,
QUERY_PARAMETER_FAILURE);
if (0 >= providerClient.delete(wordListUriBuilder.build(), null, null)) { if (0 >= providerClient.delete(wordListUriBuilder.build(), null, null)) {
Log.e(TAG, "In addition, we were unable to delete it."); Log.e(TAG, "Unable to delete a word list.");
} }
} catch (RemoteException e) { } catch (RemoteException e) {
Log.e(TAG, "In addition, communication with the dictionary provider was cut", e); Log.e(TAG, "Communication with the dictionary provider was cut", e);
return false;
} }
return true;
} }
// Ideally the two following methods should be merged, but AssetFileDescriptor does not // Ideally the two following methods should be merged, but AssetFileDescriptor does not

View File

@ -16,6 +16,7 @@
package com.android.inputmethod.latin; package com.android.inputmethod.latin;
import android.content.ContentProviderClient;
import android.content.Context; import android.content.Context;
import android.content.res.AssetFileDescriptor; import android.content.res.AssetFileDescriptor;
import android.content.res.Resources; import android.content.res.Resources;
@ -62,8 +63,12 @@ public final class DictionaryFactory {
final ReadOnlyBinaryDictionary readOnlyBinaryDictionary = final ReadOnlyBinaryDictionary readOnlyBinaryDictionary =
new ReadOnlyBinaryDictionary(f.mFilename, f.mOffset, f.mLength, new ReadOnlyBinaryDictionary(f.mFilename, f.mOffset, f.mLength,
useFullEditDistance, locale, Dictionary.TYPE_MAIN); useFullEditDistance, locale, Dictionary.TYPE_MAIN);
if (readOnlyBinaryDictionary.isValidDictionary()) { if (readOnlyBinaryDictionary.hasValidContents()) {
dictList.add(readOnlyBinaryDictionary); dictList.add(readOnlyBinaryDictionary);
} else {
readOnlyBinaryDictionary.close();
// Prevent this dictionary to do any further harm.
killDictionary(context, f);
} }
} }
} }
@ -74,6 +79,51 @@ public final class DictionaryFactory {
return new DictionaryCollection(Dictionary.TYPE_MAIN, dictList); return new DictionaryCollection(Dictionary.TYPE_MAIN, dictList);
} }
/**
* Kills a dictionary so that it is never used again, if possible.
* @param context The context to contact the dictionary provider, if possible.
* @param f A file address to the dictionary to kill.
*/
private static void killDictionary(final Context context, final AssetFileAddress f) {
if (f.pointsToPhysicalFile()) {
f.deleteUnderlyingFile();
// Warn the dictionary provider if the dictionary came from there.
final ContentProviderClient providerClient;
try {
providerClient = context.getContentResolver().acquireContentProviderClient(
BinaryDictionaryFileDumper.getProviderUriBuilder("").build());
} catch (final SecurityException e) {
Log.e(TAG, "No permission to communicate with the dictionary provider", e);
return;
}
if (null == providerClient) {
Log.e(TAG, "Can't establish communication with the dictionary provider");
return;
}
final String wordlistId =
DictionaryInfoUtils.getWordListIdFromFileName(new File(f.mFilename).getName());
if (null != wordlistId) {
// TODO: this is a reasonable last resort, but it is suboptimal.
// The following will remove the entry for this dictionary with the dictionary
// provider. When the metadata is downloaded again, we will try downloading it
// again.
// However, in the practice that will mean the user will find themselves without
// the new dictionary. That's fine for languages where it's included in the APK,
// but for other languages it will leave the user without a dictionary at all until
// the next update, which may be a few days away.
// Ideally, we would trigger a new download right away, and use increasing retry
// delays for this particular id/version combination.
// Then again, this is expected to only ever happen in case of human mistake. If
// the wrong file is on the server, the following is still doing the right thing.
// If it's a file left over from the last version however, it's not great.
BinaryDictionaryFileDumper.reportBrokenFileToDictionaryProvider(
providerClient,
context.getString(R.string.dictionary_pack_client_id),
wordlistId);
}
}
}
/** /**
* Initializes a main dictionary collection from a dictionary pack, with default flags. * Initializes a main dictionary collection from a dictionary pack, with default flags.
* *

View File

@ -44,6 +44,15 @@ public final class ReadOnlyBinaryDictionary extends Dictionary {
locale, dictType, false /* isUpdatable */); locale, dictType, false /* isUpdatable */);
} }
public boolean hasValidContents() {
mLock.readLock().lock();
try {
return mBinaryDictionary.hasValidContents();
} finally {
mLock.readLock().unlock();
}
}
public boolean isValidDictionary() { public boolean isValidDictionary() {
return mBinaryDictionary.isValidDictionary(); return mBinaryDictionary.isValidDictionary();
} }

View File

@ -135,6 +135,14 @@ static void latinime_BinaryDictionary_close(JNIEnv *env, jclass clazz, jlong dic
delete dictionary; delete dictionary;
} }
static bool latinime_BinaryDictionary_hasValidContents(JNIEnv *env, jclass clazz,
jlong dict) {
Dictionary *dictionary = reinterpret_cast<Dictionary *>(dict);
if (!dictionary) return false;
// TODO: check format version
return true;
}
static int latinime_BinaryDictionary_getFormatVersion(JNIEnv *env, jclass clazz, jlong dict) { static int latinime_BinaryDictionary_getFormatVersion(JNIEnv *env, jclass clazz, jlong dict) {
Dictionary *dictionary = reinterpret_cast<Dictionary *>(dict); Dictionary *dictionary = reinterpret_cast<Dictionary *>(dict);
if (!dictionary) return 0; if (!dictionary) return 0;
@ -437,6 +445,11 @@ static const JNINativeMethod sMethods[] = {
const_cast<char *>("(J)V"), const_cast<char *>("(J)V"),
reinterpret_cast<void *>(latinime_BinaryDictionary_close) reinterpret_cast<void *>(latinime_BinaryDictionary_close)
}, },
{
const_cast<char *>("hasValidContentsNative"),
const_cast<char *>("(J)Z"),
reinterpret_cast<void *>(latinime_BinaryDictionary_hasValidContents)
},
{ {
const_cast<char *>("getFormatVersionNative"), const_cast<char *>("getFormatVersionNative"),
const_cast<char *>("(J)I"), const_cast<char *>("(J)I"),