From aadde7d1cad5a2b147b6f909af8e9260b93e4ca4 Mon Sep 17 00:00:00 2001 From: cpfeiffer Date: Thu, 29 Dec 2016 01:41:47 +0100 Subject: [PATCH] Port to UriHelper, fix potential resource leak --- .../devices/pebble/PBWReader.java | 177 ++++++++---------- .../devices/pebble/PebbleIoThread.java | 5 +- .../gadgetbridge/util/UriHelper.java | 150 +++++++++++++++ 3 files changed, 227 insertions(+), 105 deletions(-) create mode 100644 app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/UriHelper.java diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/devices/pebble/PBWReader.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/devices/pebble/PBWReader.java index 508f3a7f..50f8ef9d 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/devices/pebble/PBWReader.java +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/devices/pebble/PBWReader.java @@ -1,19 +1,14 @@ package nodomain.freeyourgadget.gadgetbridge.devices.pebble; -import android.content.ContentResolver; import android.content.Context; -import android.database.Cursor; import android.net.Uri; -import android.provider.MediaStore; import org.json.JSONException; import org.json.JSONObject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.BufferedInputStream; import java.io.ByteArrayOutputStream; -import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; @@ -28,6 +23,7 @@ import java.util.zip.ZipInputStream; import nodomain.freeyourgadget.gadgetbridge.impl.GBDeviceApp; import nodomain.freeyourgadget.gadgetbridge.service.devices.pebble.PebbleProtocol; +import nodomain.freeyourgadget.gadgetbridge.util.UriHelper; public class PBWReader { private static final Logger LOG = LoggerFactory.getLogger(PBWReader.class); @@ -47,8 +43,7 @@ public class PBWReader { fwFileTypesMap.put("resources", PebbleProtocol.PUTBYTES_TYPE_SYSRESOURCES); } - private final Uri uri; - private final ContentResolver cr; + private final UriHelper uriHelper; private GBDeviceApp app; private ArrayList pebbleInstallables = null; private boolean isFirmware = false; @@ -59,121 +54,48 @@ public class PBWReader { private short mAppVersion; private int mIconId; private int mFlags; - private long fileSize; private JSONObject mAppKeys = null; - public PBWReader(Uri uri, Context context, String platform) throws FileNotFoundException { - this.uri = uri; - cr = context.getContentResolver(); + public PBWReader(Uri uri, Context context, String platform) throws IOException { + uriHelper = UriHelper.get(uri, context); - InputStream fin = new BufferedInputStream(cr.openInputStream(uri)); - - Uri filePathUri = uri; - if (uri.getScheme().toString().compareTo("content") == 0) { - Cursor cursor = context.getContentResolver().query(uri, null, null, null, null); - if (cursor.moveToFirst()) { - int name_index = cursor.getColumnIndexOrThrow(MediaStore.MediaColumns.DISPLAY_NAME); - int size_index = cursor.getColumnIndexOrThrow(MediaStore.MediaColumns.SIZE); - filePathUri = Uri.parse(cursor.getString(name_index)); - fileSize = cursor.getLong(size_index); - } - } - if (filePathUri.toString().endsWith(".pbl")) { + if (uriHelper.getFileName().endsWith(".pbl")) { STM32CRC stm32crc = new STM32CRC(); - try { + try (InputStream fin = uriHelper.openInputStream()) { byte[] buf = new byte[2000]; while (fin.available() > 0) { int count = fin.read(buf); stm32crc.addData(buf, count); } - fin.close(); - } catch (IOException e) { - e.printStackTrace(); - return; } - int crc = stm32crc.getResult(); // language file app = new GBDeviceApp(UUID.randomUUID(), "Language File", "unknown", "unknown", GBDeviceApp.Type.UNKNOWN); - if (fileSize == 0) { - File f = new File(uri.getPath()); - fileSize = f.length(); - } pebbleInstallables = new ArrayList<>(); - pebbleInstallables.add(new PebbleInstallable("lang", (int) fileSize, crc, PebbleProtocol.PUTBYTES_TYPE_FILE)); + pebbleInstallables.add(new PebbleInstallable("lang", (int) uriHelper.getFileSize(), crc, PebbleProtocol.PUTBYTES_TYPE_FILE)); isValid = true; isLanguage = true; return; } - String platformDir = ""; - - if (!filePathUri.toString().endsWith(".pbz")) { - /* - * for aplite and basalt it is possible to install 2.x apps which have no subfolder - * we still prefer the subfolders if present. - * chalk needs to be its subfolder - */ - String[] platformDirs; - switch (platform) { - case "basalt": - platformDirs = new String[]{"basalt/"}; - break; - case "chalk": - platformDirs = new String[]{"chalk/"}; - break; - case "diorite": - platformDirs = new String[]{"diorite/", "aplite/"}; - break; - case "emery": - platformDirs = new String[]{"emery/", "basalt/"}; - break; - default: - platformDirs = new String[]{"aplite/"}; - } - - for (String dir : platformDirs) { - InputStream afin = new BufferedInputStream(cr.openInputStream(uri)); - - ZipInputStream zis = new ZipInputStream(afin); - ZipEntry ze; - boolean found = false; - try { - while ((ze = zis.getNextEntry()) != null) { - if (ze.getName().startsWith(dir)) { - platformDir = dir; - found = true; - break; - } - } - zis.close(); - } catch (IOException e) { - e.printStackTrace(); - } - if (found) { - break; - } - } - - if (platform.equals("chalk") && platformDir.equals("")) { - return; - } + String platformDir = determinePlatformDir(uriHelper, platform); + if (platform.equals("chalk") && platformDir.equals("")) { + return; } + LOG.info("using platformdir: '" + platformDir + "'"); String appName = null; String appCreator = null; String appVersion = null; UUID appUUID = null; - ZipInputStream zis = new ZipInputStream(fin); ZipEntry ze; pebbleInstallables = new ArrayList<>(); byte[] buffer = new byte[1024]; int count; - - try { + try (ZipInputStream zis = new ZipInputStream(uriHelper.openInputStream())) { while ((ze = zis.getNextEntry()) != null) { String fileName = ze.getName(); if (fileName.equals(platformDir + "manifest.json")) { @@ -269,7 +191,6 @@ public class PBWReader { // more follows but, not interesting for us } } - zis.close(); if (appUUID != null && appName != null && appCreator != null && appVersion != null) { GBDeviceApp.Type appType = GBDeviceApp.Type.APP_GENERIC; @@ -280,11 +201,58 @@ public class PBWReader { } app = new GBDeviceApp(appUUID, appName, appCreator, appVersion, appType); } - } catch (IOException e) { - e.printStackTrace(); } } + /** + * Determines the platform dir to use for the given uri and platform. + * @param uriHelper + * @param platform + * @return the platform dir to use + * @throws IOException + */ + private String determinePlatformDir(UriHelper uriHelper, String platform) throws IOException { + String platformDir = ""; + + if (uriHelper.getFileName().endsWith(".pbz")) { + return platformDir; + } + /* + * for aplite and basalt it is possible to install 2.x apps which have no subfolder + * we still prefer the subfolders if present. + * chalk needs to be its subfolder + */ + String[] platformDirs; + switch (platform) { + case "basalt": + platformDirs = new String[]{"basalt/"}; + break; + case "chalk": + platformDirs = new String[]{"chalk/"}; + break; + case "diorite": + platformDirs = new String[]{"diorite/", "aplite/"}; + break; + case "emery": + platformDirs = new String[]{"emery/", "basalt/"}; + break; + default: + platformDirs = new String[]{"aplite/"}; + } + + for (String dir : platformDirs) { + try (ZipInputStream zis = new ZipInputStream(uriHelper.openInputStream())) { + ZipEntry ze; + while ((ze = zis.getNextEntry()) != null) { + if (ze.getName().startsWith(dir)) { + return dir; + } + } + } + } + return platformDir; + } + public boolean isFirmware() { return isFirmware; } @@ -302,28 +270,29 @@ public class PBWReader { } public InputStream getInputStreamFile(String filename) { - InputStream fin; - try { - fin = new BufferedInputStream(cr.openInputStream(uri)); - if (isLanguage) { - return fin; + if (isLanguage) { + try { + return uriHelper.openInputStream(); + } catch (FileNotFoundException e) { + LOG.warn("file not found: " + e); + return null; } - } catch (FileNotFoundException e) { - e.printStackTrace(); - return null; } - ZipInputStream zis = new ZipInputStream(fin); + ZipInputStream zis = null; ZipEntry ze; try { + zis = new ZipInputStream(uriHelper.openInputStream()); while ((ze = zis.getNextEntry()) != null) { if (ze.getName().equals(filename)) { - return zis; + return zis; // return WITHOUT closing the stream! } } zis.close(); } catch (Throwable e) { try { - zis.close(); + if (zis != null) { + zis.close(); + } } catch (IOException e1) { // ignore } diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/pebble/PebbleIoThread.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/pebble/PebbleIoThread.java index bd990d2d..035fa17b 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/pebble/PebbleIoThread.java +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/pebble/PebbleIoThread.java @@ -623,7 +623,10 @@ class PebbleIoThread extends GBDeviceIoThread { try { mPBWReader = new PBWReader(uri, getContext(), platformName); } catch (FileNotFoundException e) { - LOG.warn("file not found!"); + LOG.warn("file not found: " + e.getMessage(), e); + return; + } catch (IOException e) { + LOG.warn("unable to read file: " + e.getMessage(), e); return; } diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/UriHelper.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/UriHelper.java new file mode 100644 index 00000000..9c5d9b67 --- /dev/null +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/util/UriHelper.java @@ -0,0 +1,150 @@ +package nodomain.freeyourgadget.gadgetbridge.util; + +import android.content.ContentResolver; +import android.content.Context; +import android.database.Cursor; +import android.net.Uri; +import android.provider.MediaStore; +import android.support.annotation.NonNull; +import android.support.annotation.Nullable; + +import java.io.BufferedInputStream; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStream; + +public class UriHelper { + @NonNull + private final Uri uri; + @NonNull + private final Context context; + private String fileName; + private long fileSize; + @Nullable + private File file; + + private UriHelper(@NonNull Uri uri, @NonNull Context context) { + this.uri = uri; + this.context = context; + } + + /** + * Returns the uri as passed to #get(Uri, Context) + */ + @NonNull + public Uri getUri() { + return uri; + } + + /** + * Returns the context as passed to #get(Uri, Context) + */ + @NonNull + public Context getContext() { + return context; + } + + /** + * Returns an immutable helper to access the given Uri. In case the uri cannot be read/resolved + * an IOException is thrown. + * @param uri the uri to access + * @param context the context for accessing uris + * @throws IOException + */ + @NonNull + public static UriHelper get(@NonNull Uri uri, @NonNull Context context) throws FileNotFoundException, IOException { + UriHelper helper = new UriHelper(uri, context); + helper.resolveMetadata(); + return helper; + } + + /** + * Opens a stream to read the contents of the uri. + * Note: the caller has to close the stream after usage. + * Every invocation of this method will open a new stream. + * @throws FileNotFoundException + */ + @NonNull + public InputStream openInputStream() throws FileNotFoundException { + ContentResolver cr = context.getContentResolver(); + InputStream inputStream = cr.openInputStream(uri); + if (inputStream != null) { + return new BufferedInputStream(inputStream); + } + throw new FileNotFoundException("Unable to open inputstream for " + uri); + } + + /** + * Returns the content length (file size) in bytes + */ + public long getFileSize() { + return fileSize; + } + + /** + * Returns the name of the file referenced by the Uri. Does not include the path. + */ + @NonNull + public String getFileName() { + return fileName; + } + + /** + * Returns the file behind the uri, or null in case it is not a file:/ Uri. + * @return the file or null + */ + @Nullable + public File getFile() { + return file; + } + + private void resolveMetadata() throws IOException { + String uriScheme = uri.getScheme(); + if (ContentResolver.SCHEME_CONTENT.equals(uriScheme)) { + Cursor cursor = context.getContentResolver().query( + uri, + new String[] { + MediaStore.MediaColumns.DISPLAY_NAME, + MediaStore.MediaColumns.SIZE + }, null, null, null); + if (cursor == null) { + throw new IOException("Unable to query metadata for: " + uri); + } + if (cursor.moveToFirst()) { + int name_index = cursor.getColumnIndex(MediaStore.MediaColumns.DISPLAY_NAME); + if (name_index == -1) { + throw new IOException("Unable to retrieve name for: " + uri); + } + int size_index = cursor.getColumnIndex(MediaStore.MediaColumns.SIZE); + if (size_index == -1) { + throw new IOException("Unable to retrieve size for: " + uri); + } + try { + fileName = cursor.getString(name_index); + if (fileName == null) { + throw new IOException("Unable to retrieve name for: " + uri); + } + fileSize = cursor.getLong(size_index); + if (fileSize < 0) { + throw new IOException("Unable to retrieve size for: " + uri); + } + } catch (Exception ex) { + throw new IOException("Unable to retrieve metadata for: " + uri + ": " + ex.getMessage()); + } + } + } else if (ContentResolver.SCHEME_FILE.equals(uriScheme)) { + file = new File(uri.getPath()); + if (!file.exists()) { + throw new FileNotFoundException("Does not exist: " + file); + } + fileName = file.getName(); + fileSize = file.length(); + } else if (ContentResolver.SCHEME_ANDROID_RESOURCE.equals(uriScheme)) { + // we could actually read it, but I don't see how we can determine the file size + throw new IOException("Unsupported scheme for uri: " + uri); + } else { + throw new IOException("Unsupported scheme for uri: " + uri); + } + } +}