From 84ec6dd45833eacdc658a1b89635c9468bdc60a7 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Tue, 13 Oct 2020 17:37:14 -0400 Subject: [PATCH] Improve network reliability during resumable uploads. --- .../dependencies/ApplicationDependencies.java | 11 -- .../ApplicationDependencyProvider.java | 6 - .../securesms/keyvalue/InternalValues.java | 8 ++ .../securesms/keyvalue/SignalStore.java | 2 +- .../InternalOptionsPreferenceFragment.java | 1 + .../push/SignalServiceNetworkAccess.java | 5 + app/src/main/res/values/strings.xml | 3 + app/src/main/res/xml/preferences_internal.xml | 11 ++ .../megaphone/PinsForAllScheduleTest.java | 130 ------------------ .../internal/push/PushServiceSocket.java | 42 +++--- 10 files changed, 52 insertions(+), 167 deletions(-) delete mode 100644 app/src/test/java/org/thoughtcrime/securesms/megaphone/PinsForAllScheduleTest.java diff --git a/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencies.java b/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencies.java index 024b114cd9..7a288ba72f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencies.java +++ b/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencies.java @@ -216,16 +216,6 @@ public class ApplicationDependencies { return frameRateTracker; } - public static synchronized @NonNull KeyValueStore getKeyValueStore() { - assertInitialization(); - - if (keyValueStore == null) { - keyValueStore = provider.provideKeyValueStore(); - } - - return keyValueStore; - } - public static synchronized @NonNull MegaphoneRepository getMegaphoneRepository() { assertInitialization(); @@ -283,7 +273,6 @@ public class ApplicationDependencies { @NonNull LiveRecipientCache provideRecipientCache(); @NonNull JobManager provideJobManager(); @NonNull FrameRateTracker provideFrameRateTracker(); - @NonNull KeyValueStore provideKeyValueStore(); @NonNull MegaphoneRepository provideMegaphoneRepository(); @NonNull EarlyMessageCache provideEarlyMessageCache(); @NonNull MessageNotifier provideMessageNotifier(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencyProvider.java b/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencyProvider.java index 63d18c78bf..551742c9da 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencyProvider.java +++ b/app/src/main/java/org/thoughtcrime/securesms/dependencies/ApplicationDependencyProvider.java @@ -153,12 +153,6 @@ public class ApplicationDependencyProvider implements ApplicationDependencies.Pr return new FrameRateTracker(context); } - @Override - public @NonNull KeyValueStore provideKeyValueStore() { - return new KeyValueStore(context); - } - - @Override public @NonNull MegaphoneRepository provideMegaphoneRepository() { return new MegaphoneRepository(context); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/InternalValues.java b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/InternalValues.java index 93f4236f5c..5676123bea 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/InternalValues.java +++ b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/InternalValues.java @@ -9,6 +9,7 @@ public final class InternalValues extends SignalStoreValues { public static final String GV2_IGNORE_SERVER_CHANGES = "internal.gv2.ignore_server_changes"; public static final String GV2_IGNORE_P2P_CHANGES = "internal.gv2.ignore_p2p_changes"; public static final String RECIPIENT_DETAILS = "internal.recipient_details"; + public static final String FORCE_CENSORSHIP = "internal.force_censorship"; InternalValues(KeyValueStore store) { super(store); @@ -59,4 +60,11 @@ public final class InternalValues extends SignalStoreValues { public synchronized boolean recipientDetails() { return FeatureFlags.internalUser() && getBoolean(RECIPIENT_DETAILS, false); } + + /** + * Force the app to behave as if it is in a country where Signal is censored. + */ + public synchronized boolean forcedCensorship() { + return FeatureFlags.internalUser() && getBoolean(FORCE_CENSORSHIP, false); + } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/SignalStore.java b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/SignalStore.java index 81887e2aa1..562ba2a568 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/SignalStore.java +++ b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/SignalStore.java @@ -29,7 +29,7 @@ public final class SignalStore { private final PhoneNumberPrivacyValues phoneNumberPrivacyValues; private SignalStore() { - this.store = ApplicationDependencies.getKeyValueStore(); + this.store = new KeyValueStore(ApplicationDependencies.getApplication()); this.kbsValues = new KbsValues(store); this.registrationValues = new RegistrationValues(store); this.pinValues = new PinValues(store); diff --git a/app/src/main/java/org/thoughtcrime/securesms/preferences/InternalOptionsPreferenceFragment.java b/app/src/main/java/org/thoughtcrime/securesms/preferences/InternalOptionsPreferenceFragment.java index ebafcd4129..b2a7e77171 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/preferences/InternalOptionsPreferenceFragment.java +++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/InternalOptionsPreferenceFragment.java @@ -39,6 +39,7 @@ public class InternalOptionsPreferenceFragment extends CorrectedPreferenceFragme initializeSwitchPreference(preferenceDataStore, InternalValues.GV2_FORCE_INVITES, SignalStore.internalValues().gv2ForceInvites()); initializeSwitchPreference(preferenceDataStore, InternalValues.GV2_IGNORE_SERVER_CHANGES, SignalStore.internalValues().gv2IgnoreServerChanges()); initializeSwitchPreference(preferenceDataStore, InternalValues.GV2_IGNORE_P2P_CHANGES, SignalStore.internalValues().gv2IgnoreP2PChanges()); + initializeSwitchPreference(preferenceDataStore, InternalValues.FORCE_CENSORSHIP, SignalStore.internalValues().forcedCensorship()); findPreference("pref_refresh_attributes").setOnPreferenceClickListener(preference -> { ApplicationDependencies.getJobManager() diff --git a/app/src/main/java/org/thoughtcrime/securesms/push/SignalServiceNetworkAccess.java b/app/src/main/java/org/thoughtcrime/securesms/push/SignalServiceNetworkAccess.java index 6942dce118..641b59a862 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/push/SignalServiceNetworkAccess.java +++ b/app/src/main/java/org/thoughtcrime/securesms/push/SignalServiceNetworkAccess.java @@ -6,6 +6,7 @@ import android.content.Context; import androidx.annotation.Nullable; import org.thoughtcrime.securesms.BuildConfig; +import org.thoughtcrime.securesms.keyvalue.SignalStore; import org.thoughtcrime.securesms.net.CustomDns; import org.thoughtcrime.securesms.net.RemoteDeprecationDetectorInterceptor; import org.thoughtcrime.securesms.net.DeprecatedClientPreventionInterceptor; @@ -239,6 +240,10 @@ public class SignalServiceNetworkAccess { public SignalServiceConfiguration getConfiguration(@Nullable String localNumber) { if (localNumber == null) return this.uncensoredConfiguration; + if (SignalStore.internalValues().forcedCensorship()) { + return this.censorshipConfiguration.get(COUNTRY_CODE_QATAR); + } + for (String censoredRegion : this.censoredCountries) { if (localNumber.startsWith(censoredRegion)) { return this.censorshipConfiguration.get(censoredRegion); diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index a135d69098..a0eeea16ab 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -2231,6 +2231,9 @@ Storage service Overwrite remote data Forces remote storage to match the local device state. + Network + Force censorship + Force the app to behave as if it is in a country where Signal is censored. diff --git a/app/src/main/res/xml/preferences_internal.xml b/app/src/main/res/xml/preferences_internal.xml index 705dbb7849..eff7e8f66c 100644 --- a/app/src/main/res/xml/preferences_internal.xml +++ b/app/src/main/res/xml/preferences_internal.xml @@ -75,4 +75,15 @@ + + + + + + diff --git a/app/src/test/java/org/thoughtcrime/securesms/megaphone/PinsForAllScheduleTest.java b/app/src/test/java/org/thoughtcrime/securesms/megaphone/PinsForAllScheduleTest.java deleted file mode 100644 index 716a1c5bff..0000000000 --- a/app/src/test/java/org/thoughtcrime/securesms/megaphone/PinsForAllScheduleTest.java +++ /dev/null @@ -1,130 +0,0 @@ -package org.thoughtcrime.securesms.megaphone; - -import android.app.Application; - -import org.junit.Before; -import org.junit.Ignore; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; -import org.thoughtcrime.securesms.BaseUnitTest; -import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; -import org.thoughtcrime.securesms.keyvalue.KbsValues; -import org.thoughtcrime.securesms.keyvalue.RegistrationValues; -import org.thoughtcrime.securesms.keyvalue.SignalStore; -import org.thoughtcrime.securesms.logging.Log; -import org.thoughtcrime.securesms.util.FeatureFlags; -import org.thoughtcrime.securesms.util.TextSecurePreferences; - -import java.util.concurrent.TimeUnit; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.any; -import static org.powermock.api.mockito.PowerMockito.mock; -import static org.powermock.api.mockito.PowerMockito.mockStatic; -import static org.powermock.api.mockito.PowerMockito.when; - -@RunWith(PowerMockRunner.class) -@PrepareForTest({ApplicationDependencies.class, SignalStore.class, FeatureFlags.class, RegistrationValues.class, KbsValues.class, TextSecurePreferences.class }) -public class PinsForAllScheduleTest extends BaseUnitTest { - - private final PinsForAllSchedule testSubject = new PinsForAllSchedule(); - private final RegistrationValues registrationValues = mock(RegistrationValues.class); - private final KbsValues kbsValues = mock(KbsValues.class); - - @Before - public void setUp() throws Exception { - super.setUp(); - - mockStatic(ApplicationDependencies.class); - mockStatic(SignalStore.class); - mockStatic(FeatureFlags.class); - mockStatic(TextSecurePreferences.class); - mockStatic(Log.class); - when(ApplicationDependencies.getApplication()).thenReturn(mock(Application.class)); - when(SignalStore.registrationValues()).thenReturn(registrationValues); - when(SignalStore.kbsValues()).thenReturn(kbsValues); - when(TextSecurePreferences.isV1RegistrationLockEnabled(any())).thenReturn(false); - } - - @Test - public void givenFirstVisibleIsZero_whenIShouldDisplayFullscreen_thenIExpectFalse() { - // GIVEN - long firstVisible = 0; - - // WHEN - boolean result = PinsForAllSchedule.shouldDisplayFullScreen(firstVisible, 0); - - // THEN - assertFalse(result); - } - - @Test - public void givenFirstVisibleIsNow_whenIShouldDisplayFullscreen_thenIExpectFalse() { - // GIVEN - long now = System.currentTimeMillis(); - - // WHEN - boolean result = PinsForAllSchedule.shouldDisplayFullScreen(now, now); - - // THEN - assertFalse(result); - } - - @Test - public void givenFirstVisibleIsHalfFullscreenTimeout_whenIShouldDisplayFullscreen_thenIExpectFalse() { - // GIVEN - long now = System.currentTimeMillis(); - long lastWeek = now - TimeUnit.DAYS.toMillis(PinsForAllSchedule.DAYS_UNTIL_FULLSCREEN / 2); - - // WHEN - boolean result = PinsForAllSchedule.shouldDisplayFullScreen(lastWeek, now); - - // THEN - assertFalse(result); - } - - - // TODO [greyson] - @Ignore - @Test - public void givenFirstVisibleIsFullscreenTimeout_whenIShouldDisplayFullscreen_thenIExpectTrue() { - // GIVEN - long now = System.currentTimeMillis(); - long lastWeek = now - TimeUnit.DAYS.toMillis(PinsForAllSchedule.DAYS_UNTIL_FULLSCREEN); - - // WHEN - boolean result = PinsForAllSchedule.shouldDisplayFullScreen(lastWeek, now); - - // THEN - assertTrue(result); - } - - @Test - public void whenUserIsANewInstallAndFlagIsEnabled_whenIShouldDisplay_thenIExpectFalse() { - // GIVEN - when(registrationValues.pinWasRequiredAtRegistration()).thenReturn(true); - when(kbsValues.hasPin()).thenReturn(true); - - // WHEN - boolean result = testSubject.shouldDisplay(0, 0, 0, System.currentTimeMillis()); - - // THEN - assertFalse(result); - } - - @Test - public void whenUserIsNotANewInstallAndFlagIsEnabled_whenIShouldDisplay_thenIExpectTrue() { - // GIVEN - when(registrationValues.pinWasRequiredAtRegistration()).thenReturn(false); - - // WHEN - boolean result = testSubject.shouldDisplay(0, 0, 0, System.currentTimeMillis()); - - // THEN - assertTrue(result); - } -} \ No newline at end of file diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java index 0d598fe734..8a0dee3062 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java @@ -1108,25 +1108,10 @@ public class PushServiceSocket { .connectTimeout(soTimeoutMillis, TimeUnit.MILLISECONDS) .readTimeout(soTimeoutMillis, TimeUnit.MILLISECONDS) .build(); - final HttpUrl endpointUrl = HttpUrl.get(connectionHolder.url); - final HttpUrl signedHttpUrl; - try { - signedHttpUrl = HttpUrl.get(signedUrl); - } catch (IllegalArgumentException e) { - Log.w(TAG, "Server returned a malformed signed url: " + signedUrl); - throw new IOException("Server returned a malformed signed url", e); - } - final HttpUrl.Builder urlBuilder = new HttpUrl.Builder().scheme(endpointUrl.scheme()) - .host(endpointUrl.host()) - .port(endpointUrl.port()) - .encodedPath(endpointUrl.encodedPath()) - .addEncodedPathSegments(signedHttpUrl.encodedPath().substring(1)) - .encodedQuery(signedHttpUrl.encodedQuery()) - .encodedFragment(signedHttpUrl.encodedFragment()); - - Request.Builder request = new Request.Builder().url(urlBuilder.build()) + Request.Builder request = new Request.Builder().url(buildConfiguredUrl(connectionHolder, signedUrl)) .post(RequestBody.create(null, "")); + for (Map.Entry header : headers.entrySet()) { if (!header.getKey().equalsIgnoreCase("host")) { request.header(header.getKey(), header.getValue()); @@ -1186,7 +1171,7 @@ public class PushServiceSocket { return file.getTransmittedDigest(); } - Request.Builder request = new Request.Builder().url(resumableUrl) + Request.Builder request = new Request.Builder().url(buildConfiguredUrl(connectionHolder, resumableUrl)) .put(file) .addHeader("Content-Range", resumeInfo.contentRange); @@ -1229,7 +1214,7 @@ public class PushServiceSocket { final long offset; final String contentRange; - Request.Builder request = new Request.Builder().url(resumableUrl) + Request.Builder request = new Request.Builder().url(buildConfiguredUrl(connectionHolder, resumableUrl)) .put(RequestBody.create(null, "")) .addHeader("Content-Range", String.format(Locale.US, "bytes */%d", contentLength)); @@ -1279,6 +1264,25 @@ public class PushServiceSocket { return new ResumeInfo(contentRange, offset); } + private static HttpUrl buildConfiguredUrl(ConnectionHolder connectionHolder, String url) throws IOException { + final HttpUrl endpointUrl = HttpUrl.get(connectionHolder.url); + final HttpUrl resumableHttpUrl; + try { + resumableHttpUrl = HttpUrl.get(url); + } catch (IllegalArgumentException e) { + throw new IOException("Malformed URL!", e); + } + + return new HttpUrl.Builder().scheme(endpointUrl.scheme()) + .host(endpointUrl.host()) + .port(endpointUrl.port()) + .encodedPath(endpointUrl.encodedPath()) + .addEncodedPathSegments(resumableHttpUrl.encodedPath().substring(1)) + .encodedQuery(resumableHttpUrl.encodedQuery()) + .encodedFragment(resumableHttpUrl.encodedFragment()) + .build(); + } + private String makeServiceRequest(String urlFragment, String method, String jsonBody) throws NonSuccessfulResponseCodeException, PushNetworkException {