Karl/BobStore
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

switch etag cache check to purely client-side

Browse Source 
Instead of including the etag in the HTTP GET request and letting the
server set the Response Code depending on whether the etag machines, this
makes the client first issue a HEAD request, which is uses to get the etag
and the file size.  We need to do the HEAD beforehand anyway to get the
file size for resumable downloads, and this approach prevents the server
from using the etag as a form of tracking cookie:
http://lucb1e.com/rp/cookielesscookies/

closes #562
This commit is contained in:
Hans-Christoph Steiner 2017-09-04 16:12:20 +02:00
parent 4dd27ef727
commit 2550329ab5
6 changed files with 23 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java
View File

@ -89,11 +89,6 @@ public class IndexV1Updater extends RepoUpdater {
if (downloader.isNotFound()) {
return false;
}
if (downloader.isCached()) {
// The index is unchanged since we last read it. We just mark
// everything that came from this repo as being updated.
Utils.debugLog(TAG, "Repo index for " + dataUri + " is up to date (by etag)");
}
hasChanged = downloader.hasChanged();
if (!hasChanged) {

6
app/src/main/java/org/fdroid/fdroid/RepoUpdater.java
View File

@ -132,12 +132,6 @@ public class RepoUpdater {
downloader.setListener(downloadListener);
downloader.download();
if (downloader.isCached()) {
// The index is unchanged since we last read it. We just mark
// everything that came from this repo as being updated.
Utils.debugLog(TAG, "Repo index for " + indexUrl + " is up to date (by etag)");
}
} catch (IOException e) {
if (downloader != null && downloader.outputFile != null) {
if (!downloader.outputFile.delete()) {

8
app/src/main/java/org/fdroid/fdroid/net/BluetoothDownloader.java
View File

@ -84,14 +84,6 @@ public class BluetoothDownloader extends Downloader {
connection.closeQuietly();
}
@Override
public boolean isCached() {
FileDetails details = getFileDetails();
return details != null &&
details.getCacheTag() != null &&
details.getCacheTag().equals(getCacheTag());
}
@Override
protected void close() {
if (connection != null) {

6
app/src/main/java/org/fdroid/fdroid/net/Downloader.java
View File

@ -75,18 +75,12 @@ public abstract class Downloader {
this.cacheTag = cacheTag;
}
boolean wantToCheckCache() {
return cacheTag != null;
}
public abstract boolean hasChanged();
protected abstract int totalDownloadSize();
public abstract void download() throws IOException, InterruptedException;
public abstract boolean isCached();
/**
* @return whether the requested file was not found in the repo (e.g. HTTP 404 Not Found)
*/

67
app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java
View File

@ -1,7 +1,8 @@
package org.fdroid.fdroid.net;
import android.text.TextUtils;
import com.nostra13.universalimageloader.core.download.BaseImageDownloader;
import info.guardianproject.netcipher.NetCipher;
import org.apache.commons.io.FileUtils;
import org.fdroid.fdroid.BuildConfig;
import org.fdroid.fdroid.FDroidApp;
@ -17,18 +18,15 @@ import java.net.HttpURLConnection;
import java.net.MalformedURLException;
import java.net.URL;
import info.guardianproject.netcipher.NetCipher;
public class HttpDownloader extends Downloader {
private static final String TAG = "HttpDownloader";
private static final String HEADER_IF_NONE_MATCH = "If-None-Match";
private static final String HEADER_FIELD_ETAG = "ETag";
private final String username;
private final String password;
private HttpURLConnection connection;
private int statusCode = -1;
private boolean newFileAvailableOnServer;
HttpDownloader(URL url, File destFile)
throws FileNotFoundException, MalformedURLException {
@ -70,24 +68,33 @@ public class HttpDownloader extends Downloader {
}
/**
* Get a remote file, checking the HTTP response code. If 'etag' is not
* {@code 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.
* Get a remote file, checking the HTTP response code and the {@code etag}.
* In order to prevent the {@code etag} from being used as a form of tracking
* cookie, this code never sends the {@code etag} to the server. Instead, it
* uses a {@code HEAD} request to get the {@code etag} from the server, then
* only issues a {@code GET} if the {@code etag} has changed.
*
* @see <a href="http://lucb1e.com/rp/cookielesscookies">Cookieless cookies</a>
*/
@Override
public void download() throws IOException, InterruptedException {
// get the file size from the server
HttpURLConnection tmpConn = getConnection();
tmpConn.setRequestMethod("HEAD");
String etag = tmpConn.getHeaderField(HEADER_FIELD_ETAG);
int contentLength = -1;
int statusCode = tmpConn.getResponseCode();
tmpConn.disconnect();
newFileAvailableOnServer = false;
switch (statusCode) {
case 200:
contentLength = tmpConn.getContentLength();
if (!TextUtils.isEmpty(etag) && etag.equals(cacheTag)) {
Utils.debugLog(TAG, sourceUrl + " is cached, not downloading");
return;
}
newFileAvailableOnServer = true;
break;
case 404:
notFound = true;
@ -96,6 +103,7 @@ public class HttpDownloader extends Downloader {
Utils.debugLog(TAG, "HEAD check of " + sourceUrl + " returned " + statusCode + ": "
+ tmpConn.getResponseMessage());
}
boolean resumable = false;
long fileLength = outputFile.length();
if (fileLength > contentLength) {
@ -106,7 +114,9 @@ public class HttpDownloader extends Downloader {
resumable = true;
}
setupConnection(resumable);
doDownload(resumable);
Utils.debugLog(TAG, "downloading " + sourceUrl + " (is resumable: " + resumable + ")");
downloadFromStream(8192, resumable);
cacheTag = connection.getHeaderField(HEADER_FIELD_ETAG);
}
private boolean isSwapUrl() {
@ -147,37 +157,6 @@ public class HttpDownloader extends Downloader {
}
}
private void doDownload(boolean resumable) throws IOException, InterruptedException {
if (wantToCheckCache()) {
setupCacheCheck();
Utils.debugLog(TAG, "Checking cached status of " + sourceUrl);
statusCode = connection.getResponseCode();
}
if (isCached()) {
Utils.debugLog(TAG, sourceUrl + " is cached, so not downloading (HTTP " + statusCode + ")");
} else {
Utils.debugLog(TAG, "Need to download " + sourceUrl + " (is resumable: " + resumable + ")");
downloadFromStream(8192, resumable);
updateCacheCheck();
}
}
@Override
public boolean isCached() {
return wantToCheckCache() && statusCode == 304;
}
private void setupCacheCheck() {
if (cacheTag != null) {
connection.setRequestProperty(HEADER_IF_NONE_MATCH, cacheTag);
}
}
private void updateCacheCheck() {
cacheTag = connection.getHeaderField(HEADER_FIELD_ETAG);
}
// Testing in the emulator for me, showed that figuring out the
// filesize took about 1 to 1.5 seconds.
// To put this in context, downloading a repo of:
@ -193,7 +172,7 @@ public class HttpDownloader extends Downloader {
@Override
public boolean hasChanged() {
return this.statusCode != 304;
return newFileAvailableOnServer;
}
@Override

5
app/src/main/java/org/fdroid/fdroid/net/LocalFileDownloader.java
View File

@ -49,9 +49,4 @@ public class LocalFileDownloader extends Downloader {
notFound = true;
}
}
@Override
public boolean isCached() {
return false;
}
}