add more logging to http download connection and reset file params after setting expected size

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/entities/Message.java             |  4 
src/main/java/eu/siacs/conversations/http/HttpDownloadConnection.java  | 94 
src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java    |  1 
src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnection.java |  2 
4 files changed, 59 insertions(+), 42 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/entities/Message.java 🔗

@@ -695,6 +695,10 @@ public class Message extends AbstractEntity {
 		return isGeoUri;
 	}
 
+	public synchronized void resetFileParams() {
+		this.fileParams = null;
+	}
+
 	public synchronized FileParams getFileParams() {
 		if (fileParams == null) {
 			fileParams = new FileParams();

src/main/java/eu/siacs/conversations/http/HttpDownloadConnection.java 🔗

@@ -76,7 +76,7 @@ public class HttpDownloadConnection implements Transferable {
 			}
 			String[] parts = mUrl.getPath().toLowerCase().split("\\.");
 			String lastPart = parts.length >= 1 ? parts[parts.length - 1] : null;
-			String secondToLast = parts.length >= 2 ? parts[parts.length -2] : null;
+			String secondToLast = parts.length >= 2 ? parts[parts.length - 2] : null;
 			if ("pgp".equals(lastPart) || "gpg".equals(lastPart)) {
 				this.message.setEncryption(Message.ENCRYPTION_PGP);
 			} else if (message.getEncryption() != Message.ENCRYPTION_OTR
@@ -149,11 +149,35 @@ public class HttpDownloadConnection implements Transferable {
 			mXmppConnectionService.showErrorToastInUi(R.string.download_failed_could_not_connect);
 		} else if (e instanceof FileWriterException) {
 			mXmppConnectionService.showErrorToastInUi(R.string.download_failed_could_not_write_file);
-		} else if (!(e instanceof  CancellationException)) {
+		} else if (!(e instanceof CancellationException)) {
 			mXmppConnectionService.showErrorToastInUi(R.string.download_failed_file_not_found);
 		}
 	}
 
+	public void updateProgress(long i) {
+		this.mProgress = (int) i;
+		mHttpConnectionManager.updateConversationUi(false);
+	}
+
+	@Override
+	public int getStatus() {
+		return this.mStatus;
+	}
+
+	@Override
+	public long getFileSize() {
+		if (this.file != null) {
+			return this.file.getExpectedSize();
+		} else {
+			return 0;
+		}
+	}
+
+	@Override
+	public int getProgress() {
+		return this.mProgress;
+	}
+
 	private class FileSizeChecker implements Runnable {
 
 		private boolean interactive = false;
@@ -180,6 +204,7 @@ public class HttpDownloadConnection implements Transferable {
 				return;
 			}
 			file.setExpectedSize(size);
+			message.resetFileParams();
 			if (mHttpConnectionManager.hasStoragePermission()
 					&& size <= mHttpConnectionManager.getAutoAcceptFileSize()
 					&& mXmppConnectionService.isDataSaverDisabled()) {
@@ -203,8 +228,8 @@ public class HttpDownloadConnection implements Transferable {
 					connection = (HttpURLConnection) mUrl.openConnection();
 				}
 				connection.setRequestMethod("HEAD");
-				Log.d(Config.LOGTAG,"url: "+connection.getURL().toString());
-				Log.d(Config.LOGTAG,"connection: "+connection.toString());
+				Log.d(Config.LOGTAG, "url: " + connection.getURL().toString());
+				Log.d(Config.LOGTAG, "connection: " + connection.toString());
 				connection.setRequestProperty("User-Agent", mXmppConnectionService.getIqGenerator().getIdentityName());
 				if (connection instanceof HttpsURLConnection) {
 					mHttpConnectionManager.setupTrustManager((HttpsURLConnection) connection, interactive);
@@ -219,7 +244,7 @@ public class HttpDownloadConnection implements Transferable {
 				}
 				return Long.parseLong(contentLength, 10);
 			} catch (IOException e) {
-				Log.d(Config.LOGTAG,"io exception during HEAD "+e.getMessage());
+				Log.d(Config.LOGTAG, "io exception during HEAD " + e.getMessage());
 				throw e;
 			} catch (NumberFormatException e) {
 				throw new IOException();
@@ -258,10 +283,10 @@ public class HttpDownloadConnection implements Transferable {
 			}
 		}
 
-		private void download()  throws Exception {
+		private void download() throws Exception {
 			InputStream is = null;
 			HttpURLConnection connection = null;
-			PowerManager.WakeLock wakeLock = mHttpConnectionManager.createWakeLock("http_download_"+message.getUuid());
+			PowerManager.WakeLock wakeLock = mHttpConnectionManager.createWakeLock("http_download_" + message.getUuid());
 			try {
 				wakeLock.acquire();
 				if (mUseTor) {
@@ -272,31 +297,40 @@ public class HttpDownloadConnection implements Transferable {
 				if (connection instanceof HttpsURLConnection) {
 					mHttpConnectionManager.setupTrustManager((HttpsURLConnection) connection, interactive);
 				}
-				connection.setRequestProperty("User-Agent",mXmppConnectionService.getIqGenerator().getIdentityName());
-				final boolean tryResume = file.exists() && file.getKey() == null;
+				connection.setRequestProperty("User-Agent", mXmppConnectionService.getIqGenerator().getIdentityName());
+				final boolean tryResume = file.exists() && file.getKey() == null && file.getSize() > 0;
 				long resumeSize = 0;
+				long expected = file.getExpectedSize();
 				if (tryResume) {
-					Log.d(Config.LOGTAG,"http download trying resume");
 					resumeSize = file.getSize();
-					connection.setRequestProperty("Range", "bytes="+resumeSize+"-");
+					Log.d(Config.LOGTAG, "http download trying resume after" + resumeSize + " of " + expected);
+					connection.setRequestProperty("Range", "bytes=" + resumeSize + "-");
 				}
 				connection.setConnectTimeout(Config.SOCKET_TIMEOUT * 1000);
 				connection.setReadTimeout(Config.SOCKET_TIMEOUT * 1000);
 				connection.connect();
 				is = new BufferedInputStream(connection.getInputStream());
 				final String contentRange = connection.getHeaderField("Content-Range");
-				boolean serverResumed = tryResume && contentRange != null && contentRange.startsWith("bytes "+resumeSize+"-");
+				boolean serverResumed = tryResume && contentRange != null && contentRange.startsWith("bytes " + resumeSize + "-");
 				long transmitted = 0;
-				long expected = file.getExpectedSize();
 				if (tryResume && serverResumed) {
-					Log.d(Config.LOGTAG,"server resumed");
+					Log.d(Config.LOGTAG, "server resumed");
 					transmitted = file.getSize();
-					updateProgress((int) ((((double) transmitted) / expected) * 100));
+					updateProgress(Math.round(((double) transmitted / expected) * 100));
 					os = AbstractConnectionManager.createAppendedOutputStream(file);
 					if (os == null) {
 						throw new FileWriterException();
 					}
 				} else {
+					long reportedContentLengthOnGet;
+					try {
+						reportedContentLengthOnGet = Long.parseLong(connection.getHeaderField("Content-Length"));
+					} catch (NumberFormatException | NullPointerException e) {
+						reportedContentLengthOnGet = 0;
+					}
+					if (expected != reportedContentLengthOnGet) {
+						Log.d(Config.LOGTAG, "content-length reported on GET (" + reportedContentLengthOnGet + ") did not match Content-Length reported on HEAD (" + expected + ")");
+					}
 					file.getParentFile().mkdirs();
 					if (!file.exists() && !file.createNewFile()) {
 						throw new FileWriterException();
@@ -304,7 +338,7 @@ public class HttpDownloadConnection implements Transferable {
 					os = AbstractConnectionManager.createOutputStream(file, true);
 				}
 				int count;
-				byte[] buffer = new byte[1024];
+				byte[] buffer = new byte[4096];
 				while ((count = is.read(buffer)) != -1) {
 					transmitted += count;
 					try {
@@ -312,7 +346,7 @@ public class HttpDownloadConnection implements Transferable {
 					} catch (IOException e) {
 						throw new FileWriterException();
 					}
-					updateProgress((int) ((((double) transmitted) / expected) * 100));
+					updateProgress(Math.round(((double) transmitted / expected) * 100));
 					if (canceled) {
 						throw new CancellationException();
 					}
@@ -323,7 +357,7 @@ public class HttpDownloadConnection implements Transferable {
 					throw new FileWriterException();
 				}
 			} catch (CancellationException | IOException e) {
-				Log.d(Config.LOGTAG,"http download failed "+e.getMessage());
+				Log.d(Config.LOGTAG, "http download failed " + e.getMessage());
 				throw e;
 			} finally {
 				FileBackend.close(os);
@@ -349,28 +383,4 @@ public class HttpDownloadConnection implements Transferable {
 		}
 
 	}
-
-	public void updateProgress(int i) {
-		this.mProgress = i;
-		mHttpConnectionManager.updateConversationUi(false);
-	}
-
-	@Override
-	public int getStatus() {
-		return this.mStatus;
-	}
-
-	@Override
-	public long getFileSize() {
-		if (this.file != null) {
-			return this.file.getExpectedSize();
-		} else {
-			return 0;
-		}
-	}
-
-	@Override
-	public int getProgress() {
-		return this.mProgress;
-	}
 }

src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java 🔗

@@ -118,6 +118,7 @@ public class HttpUploadConnection implements Transferable {
 			return;
 		}
 		this.file.setExpectedSize(pair.second);
+		message.resetFileParams();
 		this.mFileInputStream = pair.first;
 		Jid host = account.getXmppConnection().findDiscoItemByFeature(Namespace.HTTP_UPLOAD);
 		IqPacket request = mXmppConnectionService.getIqGenerator().requestHttpUploadSlot(host,file,mime);

src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnection.java 🔗

@@ -460,6 +460,7 @@ public class JingleConnection implements Transferable {
 				}
 				this.mFileOutputStream = AbstractConnectionManager.createOutputStream(this.file,message.getEncryption() == Message.ENCRYPTION_AXOLOTL);
 				this.file.setExpectedSize(size);
+				message.resetFileParams();
 				Log.d(Config.LOGTAG, "receiving file: expecting size of " + this.file.getExpectedSize());
 			} else {
 				this.sendCancel();
@@ -504,6 +505,7 @@ public class JingleConnection implements Transferable {
 				cancel();
 				return;
 			}
+			message.resetFileParams();
 			this.mFileInputStream = pair.first;
 			content.setTransportId(this.transportId);
 			content.socks5transport().setChildren(getCandidatesAsElements());