Port to UriHelper, fix potential resource leak

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 UriHelper uriHelper;
-    private final ContentResolver cr;
     private GBDeviceApp app;
     private ArrayList<PebbleInstallable> 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 {
+    public PBWReader(Uri uri, Context context, String platform) throws IOException {
-        this.uri = uri;
+        uriHelper = UriHelper.get(uri, context);
-        cr = context.getContentResolver();
 
-        InputStream fin = new BufferedInputStream(cr.openInputStream(uri));
+        if (uriHelper.getFileName().endsWith(".pbl")) {
-
-        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")) {
             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 = "";
+        String platformDir = determinePlatformDir(uriHelper, platform);
-
+        if (platform.equals("chalk") && platformDir.equals("")) {
-        if (!filePathUri.toString().endsWith(".pbz")) {
+            return;
-            /*
-             * 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;
-            }
         }
+
         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 (ZipInputStream zis = new ZipInputStream(uriHelper.openInputStream())) {
-        try {
             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;
+        if (isLanguage) {
-        try {
+            try {
-            fin = new BufferedInputStream(cr.openInputStream(uri));
+                return uriHelper.openInputStream();
-            if (isLanguage) {
+            } catch (FileNotFoundException e) {
-                return fin;
+                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
             }

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;
         }
 

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);
+        }
+    }
+}