From b275a0ffff3dd9a6d0c598ce78dec268c169731b Mon Sep 17 00:00:00 2001 From: akwizgran Date: Sat, 16 Apr 2022 13:32:56 +0100 Subject: [PATCH 1/7] Increase Tor connection timeout to 2 minutes. --- .../briarproject/bramble/api/plugin/TorConstants.java | 7 +++++-- .../java/org/briarproject/bramble/socks/SocksModule.java | 3 ++- .../java/org/briarproject/bramble/socks/SocksSocket.java | 9 ++++++--- .../briarproject/bramble/socks/SocksSocketFactory.java | 9 ++++++--- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/TorConstants.java b/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/TorConstants.java index c94fc6755..10e9c557d 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/TorConstants.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/TorConstants.java @@ -1,5 +1,7 @@ package org.briarproject.bramble.api.plugin; +import static java.util.concurrent.TimeUnit.SECONDS; + public interface TorConstants { TransportId ID = new TransportId("org.briarproject.bramble.tor"); @@ -10,8 +12,9 @@ public interface TorConstants { int DEFAULT_SOCKS_PORT = 59050; int DEFAULT_CONTROL_PORT = 59051; - int CONNECT_TO_PROXY_TIMEOUT = 5000; // Milliseconds - int EXTRA_SOCKET_TIMEOUT = 30000; // Milliseconds + int CONNECT_TO_PROXY_TIMEOUT = (int) SECONDS.toMillis(5); + int EXTRA_CONNECT_TIMEOUT = (int) SECONDS.toMillis(120); + int EXTRA_SOCKET_TIMEOUT = (int) SECONDS.toMillis(30); // Local settings (not shared with contacts) String PREF_TOR_NETWORK = "network2"; diff --git a/bramble-core/src/main/java/org/briarproject/bramble/socks/SocksModule.java b/bramble-core/src/main/java/org/briarproject/bramble/socks/SocksModule.java index c74666884..5d4d7939c 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/socks/SocksModule.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/socks/SocksModule.java @@ -10,6 +10,7 @@ import dagger.Module; import dagger.Provides; import static org.briarproject.bramble.api.plugin.TorConstants.CONNECT_TO_PROXY_TIMEOUT; +import static org.briarproject.bramble.api.plugin.TorConstants.EXTRA_CONNECT_TIMEOUT; import static org.briarproject.bramble.api.plugin.TorConstants.EXTRA_SOCKET_TIMEOUT; @Module @@ -20,6 +21,6 @@ public class SocksModule { InetSocketAddress proxy = new InetSocketAddress("127.0.0.1", torSocksPort); return new SocksSocketFactory(proxy, CONNECT_TO_PROXY_TIMEOUT, - EXTRA_SOCKET_TIMEOUT); + EXTRA_CONNECT_TIMEOUT, EXTRA_SOCKET_TIMEOUT); } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/socks/SocksSocket.java b/bramble-core/src/main/java/org/briarproject/bramble/socks/SocksSocket.java index a254026ef..e7b15786e 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/socks/SocksSocket.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/socks/SocksSocket.java @@ -26,15 +26,18 @@ class SocksSocket extends Socket { "Address type not supported" }; + @SuppressWarnings("MismatchedReadAndWriteOfArray") private static final byte[] UNSPECIFIED_ADDRESS = new byte[4]; private final SocketAddress proxy; - private final int connectToProxyTimeout, extraSocketTimeout; + private final int connectToProxyTimeout; + private final int extraConnectTimeout, extraSocketTimeout; SocksSocket(SocketAddress proxy, int connectToProxyTimeout, - int extraSocketTimeout) { + int extraConnectTimeout, int extraSocketTimeout) { this.proxy = proxy; this.connectToProxyTimeout = connectToProxyTimeout; + this.extraConnectTimeout = extraConnectTimeout; this.extraSocketTimeout = extraSocketTimeout; } @@ -66,7 +69,7 @@ class SocksSocket extends Socket { // Use the supplied timeout temporarily, plus any configured extra int oldTimeout = getSoTimeout(); - setSoTimeout(timeout + extraSocketTimeout); + setSoTimeout(timeout + extraConnectTimeout); // Connect to the endpoint via the proxy sendConnectRequest(out, host, port); diff --git a/bramble-core/src/main/java/org/briarproject/bramble/socks/SocksSocketFactory.java b/bramble-core/src/main/java/org/briarproject/bramble/socks/SocksSocketFactory.java index fb0b1cd91..6f1edc8a0 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/socks/SocksSocketFactory.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/socks/SocksSocketFactory.java @@ -11,18 +11,21 @@ import javax.net.SocketFactory; class SocksSocketFactory extends SocketFactory { private final SocketAddress proxy; - private final int connectToProxyTimeout, extraSocketTimeout; + private final int connectToProxyTimeout; + private final int extraConnectTimeout, extraSocketTimeout; SocksSocketFactory(SocketAddress proxy, int connectToProxyTimeout, - int extraSocketTimeout) { + int extraConnectTimeout, int extraSocketTimeout) { this.proxy = proxy; this.connectToProxyTimeout = connectToProxyTimeout; + this.extraConnectTimeout = extraConnectTimeout; this.extraSocketTimeout = extraSocketTimeout; } @Override public Socket createSocket() { - return new SocksSocket(proxy, connectToProxyTimeout, extraSocketTimeout); + return new SocksSocket(proxy, connectToProxyTimeout, + extraConnectTimeout, extraSocketTimeout); } @Override From 96327542741f6fb350134275cb2c55a9d3bfb179 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Sat, 16 Apr 2022 19:32:51 +0100 Subject: [PATCH 2/7] Ensure task is added to queue before queue is checked. --- .../bramble/system/AndroidTaskScheduler.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java index 639824991..fd0a89c13 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java @@ -116,10 +116,12 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { long dueMillis = now + MILLISECONDS.convert(delay, unit); Runnable wakeful = () -> wakeLockManager.executeWakefully(task, executor, "TaskHandoff"); - Future check = scheduleCheckForDueTasks(delay, unit); - ScheduledTask s = new ScheduledTask(wakeful, dueMillis, check, - cancelled); + // Acquire the lock before scheduling the check to ensure the check + // doesn't access the task queue before the task has been added + ScheduledTask s; synchronized (lock) { + Future check = scheduleCheckForDueTasks(delay, unit); + s = new ScheduledTask(wakeful, dueMillis, check, cancelled); tasks.add(s); } return s; @@ -136,6 +138,7 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { return schedule(wrapped, executor, delay, unit, cancelled); } + @GuardedBy("lock") private Future scheduleCheckForDueTasks(long delay, TimeUnit unit) { Runnable wakeful = () -> wakeLockManager.runWakefully( this::runDueTasks, "TaskScheduler"); @@ -206,7 +209,7 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { private final Future check; private final AtomicBoolean cancelled; - public ScheduledTask(Runnable task, long dueMillis, + private ScheduledTask(Runnable task, long dueMillis, Future check, AtomicBoolean cancelled) { this.task = task; this.dueMillis = dueMillis; From 3c08e868228a254aa7b4eb636ac5a7fefb0678cc Mon Sep 17 00:00:00 2001 From: akwizgran Date: Sun, 17 Apr 2022 11:36:16 +0100 Subject: [PATCH 3/7] Rethrow SecurityExceptions when opening files on removable drives. --- .../plugin/file/AndroidRemovableDrivePlugin.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/file/AndroidRemovableDrivePlugin.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/file/AndroidRemovableDrivePlugin.java index 3d038f145..6d325a02f 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/file/AndroidRemovableDrivePlugin.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/file/AndroidRemovableDrivePlugin.java @@ -32,13 +32,22 @@ class AndroidRemovableDrivePlugin extends RemovableDrivePlugin { InputStream openInputStream(TransportProperties p) throws IOException { String uri = p.get(PROP_URI); if (isNullOrEmpty(uri)) throw new IllegalArgumentException(); - return app.getContentResolver().openInputStream(Uri.parse(uri)); + try { + return app.getContentResolver().openInputStream(Uri.parse(uri)); + } catch (SecurityException e) { + throw new IOException(e); + } } @Override OutputStream openOutputStream(TransportProperties p) throws IOException { String uri = p.get(PROP_URI); if (isNullOrEmpty(uri)) throw new IllegalArgumentException(); - return app.getContentResolver().openOutputStream(Uri.parse(uri), "wt"); + try { + return app.getContentResolver() + .openOutputStream(Uri.parse(uri), "wt"); + } catch (SecurityException e) { + throw new IOException(e); + } } } From c1fabcd46bb7e9b50351a17c635d82641e80b6b3 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Sun, 17 Apr 2022 11:51:49 +0100 Subject: [PATCH 4/7] Rethrow SecurityExceptions when opening images. --- .../attachment/AttachmentCreationTask.java | 18 +++++++++++------- .../android/settings/SettingsViewModel.java | 11 ++++++++--- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java index f9dd759b4..f2451de10 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java @@ -26,6 +26,7 @@ import static org.briarproject.bramble.util.IoUtils.tryToClose; import static org.briarproject.bramble.util.LogUtils.logDuration; import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.LogUtils.now; +import static org.briarproject.briar.android.attachment.media.ImageCompressor.MIME_TYPE; @NotNullByDefault class AttachmentCreationTask { @@ -100,14 +101,17 @@ class AttachmentCreationTask { if (!asList(getSupportedImageContentTypes()).contains(contentType)) { throw new UnsupportedMimeTypeException(contentType, uri); } - InputStream is = contentResolver.openInputStream(uri); - if (is == null) throw new IOException(); - is = imageCompressor - .compressImage(is, contentType); + InputStream is; + try { + is = contentResolver.openInputStream(uri); + if (is == null) throw new IOException(); + } catch (SecurityException e) { + throw new IOException(e); + } + is = imageCompressor.compressImage(is, contentType); long timestamp = System.currentTimeMillis(); - AttachmentHeader h = messagingManager - .addLocalAttachment(groupId, timestamp, - ImageCompressor.MIME_TYPE, is); + AttachmentHeader h = messagingManager.addLocalAttachment(groupId, + timestamp, MIME_TYPE, is); tryToClose(is, LOG, WARNING); logDuration(LOG, "Storing attachment", start); return h; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/settings/SettingsViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/settings/SettingsViewModel.java index 694460934..df1788712 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/settings/SettingsViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/settings/SettingsViewModel.java @@ -231,9 +231,14 @@ class SettingsViewModel extends DbViewModel implements EventListener { if (!asList(getSupportedImageContentTypes()).contains(contentType)) { throw new UnsupportedMimeTypeException(contentType, uri); } - InputStream is = contentResolver.openInputStream(uri); - if (is == null) throw new IOException( - "ContentResolver returned null when opening InputStream"); + InputStream is; + try { + is = contentResolver.openInputStream(uri); + if (is == null) throw new IOException( + "ContentResolver returned null when opening InputStream"); + } catch (SecurityException e) { + throw new IOException(e); + } InputStream compressed = imageCompressor.compressImage(is, contentType); runOnDbThread(() -> { From bc013296f62c707a71f4bee4e0a1b4e4a3607883 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Sun, 17 Apr 2022 11:59:00 +0100 Subject: [PATCH 5/7] Catch ActivityNotFoundException when saving image. --- .../briar/android/conversation/ImageActivity.java | 7 ++++++- .../briar/android/conversation/ImageViewModel.java | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageActivity.java index 3a0b289a0..641b1cf90 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageActivity.java @@ -1,5 +1,6 @@ package org.briarproject.briar.android.conversation; +import android.content.ActivityNotFoundException; import android.content.DialogInterface.OnClickListener; import android.content.Intent; import android.net.Uri; @@ -263,7 +264,11 @@ public class ImageActivity extends BriarActivity if (SDK_INT >= 19) { String name = viewModel.getFileName() + "." + getVisibleAttachment().getExtension(); - launcher.launch(name); + try { + launcher.launch(name); + } catch (ActivityNotFoundException e) { + viewModel.onSaveImageError(); + } } else { viewModel.saveImage(getVisibleAttachment()); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java index 823151fca..5260785af 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java @@ -186,6 +186,11 @@ public class ImageViewModel extends DbViewModel implements EventListener { } } + @UiThread + void onSaveImageError() { + saveState.setEvent(true); + } + /** * Saves the attachment on external storage, * assuming the permission was granted during install time. From d3c7832245b59eb33a43e425dc21139d74b88d36 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 18 Apr 2022 11:33:47 +0100 Subject: [PATCH 6/7] Update introduction onboarding text. The old text caused some confusion in user testing because contacts can now add each other remotely. --- briar-android/src/main/res/values/strings.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml index eb97d2ba0..f3d2ec0ab 100644 --- a/briar-android/src/main/res/values/strings.xml +++ b/briar-android/src/main/res/values/strings.xml @@ -320,7 +320,7 @@ Introduce your contacts - You can introduce your contacts to each other, so they don\'t need to meet in person to connect on Briar. + Introduce your contacts to each other so they can connect on Briar. Make Introduction Select Contact You already have one introduction in progress with these contacts. Please allow for this to finish first. If you or your contacts are rarely online, this can take some time. From 961af66c8e4b324179231ff8ad904f32d62499af Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 18 Apr 2022 13:33:09 +0100 Subject: [PATCH 7/7] Use new onSaveImageError() method for readability. --- .../briarproject/briar/android/conversation/ImageViewModel.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java index 5260785af..b8a7ff902 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java @@ -180,7 +180,7 @@ public class ImageViewModel extends DbViewModel implements EventListener { @UiThread void saveImage(AttachmentItem attachment, @Nullable Uri uri) { if (uri == null) { - saveState.setEvent(true); + onSaveImageError(); } else { saveImage(attachment, () -> getOutputStream(uri), null); }