From 407c903010af26cc85d28cdc36b3ccc008ed226d Mon Sep 17 00:00:00 2001 From: Ciaran Gultnieks Date: Sat, 22 Sep 2012 22:33:06 +0100 Subject: [PATCH] Use etags - highly experimental, especially where multiple repos are concerned --- src/org/fdroid/fdroid/AppDetails.java | 2 +- src/org/fdroid/fdroid/DB.java | 70 +++++--- src/org/fdroid/fdroid/Downloader.java | 2 +- src/org/fdroid/fdroid/RepoXMLHandler.java | 209 +++++++++++++--------- src/org/fdroid/fdroid/UpdateService.java | 66 +++++-- 5 files changed, 226 insertions(+), 123 deletions(-) diff --git a/src/org/fdroid/fdroid/AppDetails.java b/src/org/fdroid/fdroid/AppDetails.java index 60f7b315a..fec90e1c3 100644 --- a/src/org/fdroid/fdroid/AppDetails.java +++ b/src/org/fdroid/fdroid/AppDetails.java @@ -307,7 +307,7 @@ public class AppDetails extends ListActivity { // Make sure the app is populated. try { DB db = DB.getDB(); - db.populateDetails(app); + db.populateDetails(app, null); } catch (Exception ex) { Log.d("FDroid", "Failed to populate app - " + ex.getMessage()); } finally { diff --git a/src/org/fdroid/fdroid/DB.java b/src/org/fdroid/fdroid/DB.java index 6d1fc3b68..da3c38f95 100644 --- a/src/org/fdroid/fdroid/DB.java +++ b/src/org/fdroid/fdroid/DB.java @@ -239,7 +239,7 @@ public class DB { detail_size = 0; apkSource = null; added = null; - detail_server = null; + server = null; detail_hash = null; detail_hashType = null; detail_permissions = null; @@ -250,7 +250,7 @@ public class DB { public String version; public int vercode; public int detail_size; // Size in bytes - 0 means we don't know! - public String detail_server; + public String server; public String detail_hash; public String detail_hashType; public int minSdkVersion; // 0 if unknown @@ -282,7 +282,7 @@ public class DB { public String getURL() { String path = apkName.replace(" ", "%20"); - return detail_server + "/" + path; + return server + "/" + path; } // Call isCompatible(apk) on an instance of this class to @@ -364,16 +364,17 @@ public class DB { private static final String CREATE_TABLE_REPO = "create table " + TABLE_REPO + " (" + "address text primary key, " + "inuse integer not null, " + "priority integer not null," - + "pubkey text);"; + + "pubkey text, lastetag text);"; public static class Repo { public String address; public boolean inuse; public int priority; public String pubkey; // null for an unsigned repo + public String lastetag; // last etag we updated from, null forces update } - private final int DBVersion = 18; + private final int DBVersion = 19; private static void createAppApk(SQLiteDatabase db) { db.execSQL(CREATE_TABLE_APP); @@ -409,6 +410,7 @@ public class DB { mContext.getString(R.string.default_repo_pubkey)); values.put("inuse", 1); values.put("priority", 10); + values.put("lastetag", (String) null); db.insert(TABLE_REPO, null, values); } @@ -417,6 +419,8 @@ public class DB { resetTransient(db); if (oldVersion < 7) db.execSQL("alter table " + TABLE_REPO + " add pubkey string"); + if (oldVersion < 19) + db.execSQL("alter table " + TABLE_REPO + " add lastetag string"); } } @@ -520,7 +524,9 @@ public class DB { } // Populate the details for the given app, if necessary. - public void populateDetails(App app) { + // If 'apkrepo' is not null, only apks from that repo address are + // populated (this is used during the update process) + public void populateDetails(App app, String apkrepo) { if (app.detail_Populated) return; Cursor c = null; @@ -538,24 +544,22 @@ public class DB { c.close(); c = null; - cols = new String[] { "server", "hash", "hashType", "size", - "permissions" }; + cols = new String[] { "hash", "hashType", "size", "permissions" }; for (Apk apk : app.apks) { - c = db.query( - TABLE_APK, - cols, - "id = ? and vercode = " + Integer.toString(apk.vercode), - new String[] { apk.id }, null, null, null, null); - c.moveToFirst(); - apk.detail_server = c.getString(0); - apk.detail_hash = c.getString(1); - apk.detail_hashType = c.getString(2); - apk.detail_size = c.getInt(3); - apk.detail_permissions = CommaSeparatedList - .make(c.getString(4)); - c.close(); - c = null; + if (apkrepo == null || apkrepo.equals(apk.server)) { + c = db.query(TABLE_APK, cols, "id = ? and vercode = " + + Integer.toString(apk.vercode), + new String[] { apk.id }, null, null, null, null); + c.moveToFirst(); + apk.detail_hash = c.getString(0); + apk.detail_hashType = c.getString(1); + apk.detail_size = c.getInt(2); + apk.detail_permissions = CommaSeparatedList.make(c + .getString(3)); + c.close(); + c = null; + } } app.detail_Populated = true; @@ -636,7 +640,7 @@ public class DB { cols = new String[] { "id", "version", "vercode", "sig", "srcname", "apkName", "apkSource", "minSdkVersion", "added", - "features", "compatible" }; + "features", "compatible", "server" }; c = db.query(TABLE_APK, cols, null, null, null, null, "vercode desc"); c.moveToFirst(); @@ -655,6 +659,7 @@ public class DB { : mDateFormat.parse(sApkAdded); apk.features = CommaSeparatedList.make(c.getString(9)); apk.compatible = c.getInt(10) == 1; + apk.server = c.getString(11); apps.get(apk.id).apks.add(apk); c.moveToNext(); } @@ -945,7 +950,7 @@ public class DB { values.put("id", upapk.id); values.put("version", upapk.version); values.put("vercode", upapk.vercode); - values.put("server", upapk.detail_server); + values.put("server", upapk.server); values.put("hash", upapk.detail_hash); values.put("hashType", upapk.detail_hashType); values.put("sig", upapk.sig); @@ -974,8 +979,9 @@ public class DB { Vector repos = new Vector(); Cursor c = null; try { - c = db.rawQuery("select address, inuse, priority, pubkey from " - + TABLE_REPO + " order by priority", null); + c = db.rawQuery( + "select address, inuse, priority, pubkey, lastetag from " + + TABLE_REPO + " order by priority", null); c.moveToFirst(); while (!c.isAfterLast()) { Repo repo = new Repo(); @@ -983,6 +989,7 @@ public class DB { repo.inuse = (c.getInt(1) == 1); repo.priority = c.getInt(2); repo.pubkey = c.getString(3); + repo.lastetag = c.getString(4); repos.add(repo); c.moveToNext(); } @@ -997,7 +1004,7 @@ public class DB { public void changeServerStatus(String address) { db.execSQL("update " + TABLE_REPO - + " set inuse=1-inuse where address = ?", + + " set inuse=1-inuse, lastetag=null where address = ?", new String[] { address }); } @@ -1006,6 +1013,14 @@ public class DB { values.put("inuse", repo.inuse); values.put("priority", repo.priority); values.put("pubkey", repo.pubkey); + values.put("lastetag", (String) null); + db.update(TABLE_REPO, values, "address = ?", + new String[] { repo.address }); + } + + public void writeLastEtag(Repo repo) { + ContentValues values = new ContentValues(); + values.put("lastetag", repo.lastetag); db.update(TABLE_REPO, values, "address = ?", new String[] { repo.address }); } @@ -1016,6 +1031,7 @@ public class DB { values.put("inuse", 1); values.put("priority", priority); values.put("pubkey", pubkey); + values.put("lastetag", (String) null); db.insert(TABLE_REPO, null, values); } diff --git a/src/org/fdroid/fdroid/Downloader.java b/src/org/fdroid/fdroid/Downloader.java index 74503b1ef..2be8c9312 100644 --- a/src/org/fdroid/fdroid/Downloader.java +++ b/src/org/fdroid/fdroid/Downloader.java @@ -117,7 +117,7 @@ public class Downloader extends Thread { // If we haven't got the apk locally, we'll have to download it... String remotefile; if (curapk.apkSource == null) { - remotefile = curapk.detail_server + "/" + apkname.replace(" ", "%20"); + remotefile = curapk.server + "/" + apkname.replace(" ", "%20"); } else { remotefile = curapk.apkSource; } diff --git a/src/org/fdroid/fdroid/RepoXMLHandler.java b/src/org/fdroid/fdroid/RepoXMLHandler.java index d19d68ca4..04a2d92be 100644 --- a/src/org/fdroid/fdroid/RepoXMLHandler.java +++ b/src/org/fdroid/fdroid/RepoXMLHandler.java @@ -28,6 +28,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; +import java.net.HttpURLConnection; import java.net.MalformedURLException; import java.net.URL; import java.security.cert.Certificate; @@ -237,7 +238,7 @@ public class RepoXMLHandler extends DefaultHandler { } else if (localName == "package" && curapp != null && curapk == null) { curapk = new DB.Apk(); curapk.id = curapp.id; - curapk.detail_server = server; + curapk.server = server; hashType = null; } else if (localName == "hash" && curapk != null) { hashType = attributes.getValue("", "type"); @@ -245,24 +246,44 @@ public class RepoXMLHandler extends DefaultHandler { curchars.setLength(0); } + // Get a remote file. Returns the HTTP response code. + // If 'etag' is not null, it's passed to the server as an If-None-Match + // header, in which case expect a 304 response if nothing changed. + // In the event of a 200 response ONLY, 'retag' (which should be passed + // empty) may contain an etag value for the response, or it may be left + // empty if none was available. + private static int getRemoteFile(Context ctx, String url, String dest, + String etag, StringBuilder retag) throws MalformedURLException, + IOException { - private static void getRemoteFile(Context ctx, String url, String dest) - throws MalformedURLException, IOException { - FileOutputStream f = ctx.openFileOutput(dest, Context.MODE_PRIVATE); + URL u = new URL(url); + HttpURLConnection uc = (HttpURLConnection) u.openConnection(); + if (etag != null) + uc.setRequestProperty("If-None-Match", etag); + int code = uc.getResponseCode(); + if (code == 200) { - BufferedInputStream getit = new BufferedInputStream( - new URL(url).openStream()); - BufferedOutputStream bout = new BufferedOutputStream(f, 1024); - byte data[] = new byte[1024]; + FileOutputStream f = ctx.openFileOutput(dest, Context.MODE_PRIVATE); - int readed = getit.read(data, 0, 1024); - while (readed != -1) { - bout.write(data, 0, readed); - readed = getit.read(data, 0, 1024); + BufferedInputStream getit = new BufferedInputStream( + new URL(url).openStream()); + BufferedOutputStream bout = new BufferedOutputStream(f, 1024); + byte data[] = new byte[1024]; + + int readed = getit.read(data, 0, 1024); + while (readed != -1) { + bout.write(data, 0, readed); + readed = getit.read(data, 0, 1024); + } + bout.close(); + getit.close(); + f.close(); + + String et = uc.getHeaderField("ETag"); + if (et != null) + retag.append(et); } - bout.close(); - getit.close(); - f.close(); + return code; } @@ -271,9 +292,14 @@ public class RepoXMLHandler extends DefaultHandler { // APKs are merged into the existing one). // Returns null if successful, otherwise an error message to be displayed // to the user (if there is an interactive user!) - public static String doUpdate(Context ctx, DB.Repo repo, Vector apps) { + // 'newetag' should be passed empty. On success, it may contain an etag + // value for the index that was successfully processed, or it may contain + // null if none was available. + public static String doUpdate(Context ctx, DB.Repo repo, + Vector apps, StringBuilder newetag, Vector keeprepos) { try { + int code = 0; if (repo.pubkey != null) { // This is a signed repo - we download the jar file, @@ -286,85 +312,106 @@ public class RepoXMLHandler extends DefaultHandler { address += "?" + pi.versionName; } catch (Exception e) { } - getRemoteFile(ctx, address, "tempindex.jar"); - String jarpath = ctx.getFilesDir() + "/tempindex.jar"; - JarFile jar; - JarEntry je; - try { - jar = new JarFile(jarpath, true); - je = (JarEntry) jar.getEntry("index.xml"); - File efile = new File(ctx.getFilesDir(), "/tempindex.xml"); - InputStream in = new BufferedInputStream( - jar.getInputStream(je), 8192); - OutputStream out = new BufferedOutputStream( - new FileOutputStream(efile), 8192); - byte[] buffer = new byte[8192]; - while (true) { - int nBytes = in.read(buffer); - if (nBytes <= 0) - break; - out.write(buffer, 0, nBytes); + code = getRemoteFile(ctx, address, "tempindex.jar", + repo.lastetag, newetag); + if (code == 200) { + String jarpath = ctx.getFilesDir() + "/tempindex.jar"; + JarFile jar; + JarEntry je; + try { + jar = new JarFile(jarpath, true); + je = (JarEntry) jar.getEntry("index.xml"); + File efile = new File(ctx.getFilesDir(), + "/tempindex.xml"); + InputStream in = new BufferedInputStream( + jar.getInputStream(je), 8192); + OutputStream out = new BufferedOutputStream( + new FileOutputStream(efile), 8192); + byte[] buffer = new byte[8192]; + while (true) { + int nBytes = in.read(buffer); + if (nBytes <= 0) + break; + out.write(buffer, 0, nBytes); + } + out.flush(); + out.close(); + in.close(); + } catch (SecurityException e) { + Log.e("FDroid", "Invalid hash for index file"); + return "Invalid hash for index file"; } - out.flush(); - out.close(); - in.close(); - } catch (SecurityException e) { - Log.e("FDroid", "Invalid hash for index file"); - return "Invalid hash for index file"; - } - Certificate[] certs = je.getCertificates(); - jar.close(); - if (certs == null) { - Log.d("FDroid", "No signature found in index"); - return "No signature found in index"; - } - Log.d("FDroid", "Index has " + certs.length + " signature" - + (certs.length > 1 ? "s." : ".")); + Certificate[] certs = je.getCertificates(); + jar.close(); + if (certs == null) { + Log.d("FDroid", "No signature found in index"); + return "No signature found in index"; + } + Log.d("FDroid", "Index has " + certs.length + " signature" + + (certs.length > 1 ? "s." : ".")); - boolean match = false; - for (Certificate cert : certs) { - String certdata = Hasher.hex(cert.getEncoded()); - if (repo.pubkey.equals(certdata)) { - match = true; - break; + boolean match = false; + for (Certificate cert : certs) { + String certdata = Hasher.hex(cert.getEncoded()); + if (repo.pubkey.equals(certdata)) { + match = true; + break; + } + } + if (!match) { + Log.d("FDroid", "Index signature mismatch"); + return "Index signature mismatch"; } - } - if (!match) { - Log.d("FDroid", "Index signature mismatch"); - return "Index signature mismatch"; } } else { // It's an old-fashioned unsigned repo... Log.d("FDroid", "Getting unsigned index from " + repo.address); - getRemoteFile(ctx, repo.address + "/index.xml", "tempindex.xml"); + code = getRemoteFile(ctx, repo.address + "/index.xml", + "tempindex.xml", repo.lastetag, newetag); } - // Process the index... - SAXParserFactory spf = SAXParserFactory.newInstance(); - SAXParser sp = spf.newSAXParser(); - XMLReader xr = sp.getXMLReader(); - RepoXMLHandler handler = new RepoXMLHandler(repo.address, apps); - xr.setContentHandler(handler); + if (code == 200) { + // Process the index... + SAXParserFactory spf = SAXParserFactory.newInstance(); + SAXParser sp = spf.newSAXParser(); + XMLReader xr = sp.getXMLReader(); + RepoXMLHandler handler = new RepoXMLHandler(repo.address, apps); + xr.setContentHandler(handler); - InputStreamReader isr = new FileReader(new File(ctx.getFilesDir() - + "/tempindex.xml")); - InputSource is = new InputSource(isr); - xr.parse(is); + InputStreamReader isr = new FileReader(new File( + ctx.getFilesDir() + "/tempindex.xml")); + InputSource is = new InputSource(isr); + xr.parse(is); - if (handler.pubkey != null && repo.pubkey == null) { - // We read an unsigned index, but that indicates that - // a signed version is now available... - Log.d("FDroid", - "Public key found - switching to signed repo for future updates"); - repo.pubkey = handler.pubkey; - DB db = DB.getDB(); - try { - db.updateRepoByAddress(repo); - } finally { - DB.releaseDB(); + if (handler.pubkey != null && repo.pubkey == null) { + // We read an unsigned index, but that indicates that + // a signed version is now available... + Log.d("FDroid", + "Public key found - switching to signed repo for future updates"); + repo.pubkey = handler.pubkey; + try { + DB db = DB.getDB(); + db.updateRepoByAddress(repo); + } finally { + DB.releaseDB(); + } } + + } else if (code == 304) { + // The index is unchanged since we last read it. We just mark + // everything that came from this repo as being updated. + Log.d("FDroid", "Repo index for " + repo.address + + " is up to date (by etag)"); + keeprepos.add(repo.address); + // Make sure we give back the same etag. (The 200 route will + // have supplied a new one. + newetag.append(repo.lastetag); + + } else { + return "Failed to read index - HTTP response " + + Integer.toString(code); } } catch (SSLHandshakeException sslex) { diff --git a/src/org/fdroid/fdroid/UpdateService.java b/src/org/fdroid/fdroid/UpdateService.java index 9ea697166..9d3961998 100644 --- a/src/org/fdroid/fdroid/UpdateService.java +++ b/src/org/fdroid/fdroid/UpdateService.java @@ -102,13 +102,12 @@ public class UpdateService extends IntentService { boolean notify = prefs.getBoolean("updateNotify", false); // Grab some preliminary information, then we can release the - // database - // while we do all the downloading, etc... - DB db = DB.getDB(); + // database while we do all the downloading, etc... int prevUpdates = 0; int newUpdates = 0; Vector repos; try { + DB db = DB.getDB(); repos = db.getRepos(); } finally { DB.releaseDB(); @@ -116,12 +115,16 @@ public class UpdateService extends IntentService { // Process each repo... Vector apps = new Vector(); + Vector keeprepos = new Vector(); boolean success = true; for (DB.Repo repo : repos) { if (repo.inuse) { + StringBuilder newetag = new StringBuilder(); String err = RepoXMLHandler.doUpdate(getBaseContext(), - repo, apps); - if (err != null) { + repo, apps, newetag, keeprepos); + if (err == null) { + repo.lastetag = newetag.toString(); + } else { success = false; err = "Update failed for " + repo.address + " - " + err; Log.d("FDroid", err); @@ -137,16 +140,55 @@ public class UpdateService extends IntentService { Vector acceptedapps = new Vector(); Vector prevapps = ((FDroidApp) getApplication()) .getApps(); - db = DB.getDB(); + + DB db = DB.getDB(); try { + + // Need to flag things we're keeping despite having received + // no data about during the update. (i.e. stuff from a repo + // that we know is unchanged due to the etag) + for (String keep : keeprepos) { + for (DB.App app : prevapps) { + boolean keepapp = false; + for (DB.Apk apk : app.apks) { + if (apk.server.equals(keep)) { + keepapp = true; + break; + } + } + if (keepapp) { + DB.App app_k = null; + for (DB.App app2 : apps) { + if (app2.id.equals(app.id)) { + app_k = app2; + break; + } + } + if (app_k == null) { + apps.add(app); + app_k = app; + } + app_k.updated = true; + if (!app_k.detail_Populated) { + db.populateDetails(app_k, keep); + } + for (DB.Apk apk : app.apks) + if (apk.server.equals(keep)) + apk.updated = true; + } + } + } + prevUpdates = db.beginUpdate(prevapps); for (DB.App app : apps) { - if(db.updateApplication(app)) + if (db.updateApplication(app)) acceptedapps.add(app); } db.endUpdate(); if (notify) newUpdates = db.getNumUpdates(); + for (DB.Repo repo : repos) + db.writeLastEtag(repo); } catch (Exception ex) { db.cancelUpdate(); Log.e("FDroid", "Exception during update processing:\n" @@ -217,7 +259,7 @@ public class UpdateService extends IntentService { } } - + private void getIcon(DB.App app) { try { @@ -225,9 +267,9 @@ public class UpdateService extends IntentService { if (f.exists()) return; - if(app.apks.size() == 0) + if (app.apks.size() == 0) return; - String server = app.apks.get(0).detail_server; + String server = app.apks.get(0).server; URL u = new URL(server + "/icons/" + app.icon); HttpURLConnection uc = (HttpURLConnection) u.openConnection(); if (uc.getResponseCode() == 200) { @@ -251,7 +293,5 @@ public class UpdateService extends IntentService { } } - + } - -