summaryrefslogtreecommitdiff
path: root/app/src
diff options
context:
space:
mode:
authorcyBerta <cyberta@riseup.net>2017-11-03 15:17:45 +0100
committercyBerta <cyberta@riseup.net>2017-11-03 15:17:45 +0100
commit859c2b4496618c8142c70610ebba395e9aa4d5dc (patch)
tree137fd4af0292e8b8a8abb26307f82c580f485a0f /app/src
parent719c63ee33a88b239e96efead8da14cf985ca9aa (diff)
#8778 fixes invalid provider error handling
Diffstat (limited to 'app/src')
-rw-r--r--app/src/androidTest/java/se/leap/bitmaskclient/test/TestConfigurationWizard.java36
-rw-r--r--app/src/insecure/java/se/leap/bitmaskclient/ProviderAPI.java12
-rw-r--r--app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java2
-rw-r--r--app/src/main/java/se/leap/bitmaskclient/ProviderApiBase.java11
-rw-r--r--app/src/production/java/se/leap/bitmaskclient/ProviderAPI.java12
5 files changed, 57 insertions, 16 deletions
diff --git a/app/src/androidTest/java/se/leap/bitmaskclient/test/TestConfigurationWizard.java b/app/src/androidTest/java/se/leap/bitmaskclient/test/TestConfigurationWizard.java
index 90b8b839..12e88af8 100644
--- a/app/src/androidTest/java/se/leap/bitmaskclient/test/TestConfigurationWizard.java
+++ b/app/src/androidTest/java/se/leap/bitmaskclient/test/TestConfigurationWizard.java
@@ -14,7 +14,6 @@ import se.leap.bitmaskclient.R;
public class TestConfigurationWizard extends ActivityInstrumentationTestCase2<ConfigurationWizard> {
private Solo solo;
- private static int added_providers;
public TestConfigurationWizard() {
super(ConfigurationWizard.class);
@@ -34,11 +33,14 @@ public class TestConfigurationWizard extends ActivityInstrumentationTestCase2<Co
super.tearDown();
}
- public void testListProviders() {
+ /**
+ * Tests should run independently from each other. We need a better approach to test the amount of providers added
+ */
+ /*public void testListProviders() {
assertEquals(solo.getCurrentViews(ListView.class).size(), 1);
assertEquals("Number of available providers differ", predefinedProviders() + added_providers, shownProviders());
- }
+ }*/
private int shownProviders() {
return solo.getCurrentViews(ListView.class).get(0).getCount();
@@ -76,24 +78,40 @@ public class TestConfigurationWizard extends ActivityInstrumentationTestCase2<Co
public void testAddNewProvider() {
//addProvider("calyx.net");
- addProvider("riseup.net");
+ addProvider("riseup.net", true);
+ }
+
+ public void testAddFalseProviderReturning404() {
+ //addProvider("calyx.net");
+ addProvider("startpage.com", false);
+ }
+
+ public void testAddFalseProviderReturning200() {
+ //addProvider("calyx.net");
+ addProvider("test.com", false);
}
- private void addProvider(String url) {
- boolean is_new_provider = !solo.searchText(url);
+ private void addProvider(String url, boolean expectSuccess) {
- if (is_new_provider)
- added_providers = added_providers + 1;
solo.clickOnActionBarItem(R.id.new_provider);
solo.enterText(0, url);
if ( BuildConfig.FLAVOR.equals("insecure")) {
solo.clickOnCheckBox(0);
}
solo.clickOnText(solo.getString(R.string.save));
- waitForProviderDetails();
+ if (expectSuccess) {
+ waitForProviderDetails();
+ } else {
+ waitForNoValidProviderError();
+ }
solo.goBack();
}
+ private void waitForNoValidProviderError() {
+ String text = solo.getString(R.string.malformed_url);
+ assertTrue("Provider details dialog did not appear", solo.waitForText(text, 1, 60*1000));
+ }
+
public void testShowAbout() {
showAbout();
}
diff --git a/app/src/insecure/java/se/leap/bitmaskclient/ProviderAPI.java b/app/src/insecure/java/se/leap/bitmaskclient/ProviderAPI.java
index 588ff7e2..87fff283 100644
--- a/app/src/insecure/java/se/leap/bitmaskclient/ProviderAPI.java
+++ b/app/src/insecure/java/se/leap/bitmaskclient/ProviderAPI.java
@@ -125,6 +125,12 @@ public class ProviderAPI extends ProviderApiBase {
else
provider_dot_json_string = downloadWithCommercialCA(provider_main_url + "/provider.json", danger_on, provider_ca_cert_fingerprint);
+ if (!isValidJson(provider_dot_json_string)) {
+ result.putString(ERRORS, getString(malformed_url));
+ result.putBoolean(RESULT_KEY, false);
+ return result;
+ }
+
try {
JSONObject provider_json = new JSONObject(provider_dot_json_string);
provider_api_url = provider_json.getString(Provider.API_URL) + "/" + provider_json.getString(Provider.API_VERSION);
@@ -165,7 +171,7 @@ public class ProviderAPI extends ProviderApiBase {
preferences.edit().putString(Constants.KEY, eip_service_json.toString()).commit();
result.putBoolean(RESULT_KEY, true);
- } catch (JSONException e) {
+ } catch (NullPointerException | JSONException e) {
String reason_to_fail = pickErrorMessage(eip_service_json_string);
result.putString(ERRORS, reason_to_fail);
result.putBoolean(RESULT_KEY, false);
@@ -189,7 +195,7 @@ public class ProviderAPI extends ProviderApiBase {
String cert_string = downloadWithProviderCA(new_cert_string_url.toString(), last_danger_on);
- if (cert_string.isEmpty() || ConfigHelper.checkErroneousDownload(cert_string))
+ if (cert_string == null || cert_string.isEmpty() || ConfigHelper.checkErroneousDownload(cert_string))
return false;
else
return loadCertificate(cert_string);
@@ -274,7 +280,7 @@ public class ProviderAPI extends ProviderApiBase {
responseString = sendGetStringToServer(string_url, headerArgs, okHttpClient);
- if (responseString.contains(ERRORS)) {
+ if (responseString != null && responseString.contains(ERRORS)) {
try {
// try to download with provider CA on certificate error
JSONObject responseErrorJson = new JSONObject(responseString);
diff --git a/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java b/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java
index 4929f040..33b2e225 100644
--- a/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java
+++ b/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java
@@ -47,7 +47,7 @@ public class ConfigHelper {
} else {
return false;
}
- } catch (JSONException e) {
+ } catch (NullPointerException | JSONException e) {
return false;
}
}
diff --git a/app/src/main/java/se/leap/bitmaskclient/ProviderApiBase.java b/app/src/main/java/se/leap/bitmaskclient/ProviderApiBase.java
index caa48231..bf1c3941 100644
--- a/app/src/main/java/se/leap/bitmaskclient/ProviderApiBase.java
+++ b/app/src/main/java/se/leap/bitmaskclient/ProviderApiBase.java
@@ -687,6 +687,17 @@ public abstract class ProviderApiBase extends IntentService {
return CA_CERT_DOWNLOADED;
}
+ protected boolean isValidJson(String jsonString) {
+ try {
+ new JSONObject(jsonString);
+ return true;
+ } catch(JSONException e) {
+ return false;
+ } catch(NullPointerException e) {
+ return false;
+ }
+ }
+
protected boolean validCertificate(String cert_string) {
boolean result = false;
if (!ConfigHelper.checkErroneousDownload(cert_string)) {
diff --git a/app/src/production/java/se/leap/bitmaskclient/ProviderAPI.java b/app/src/production/java/se/leap/bitmaskclient/ProviderAPI.java
index 982080de..70a2c27c 100644
--- a/app/src/production/java/se/leap/bitmaskclient/ProviderAPI.java
+++ b/app/src/production/java/se/leap/bitmaskclient/ProviderAPI.java
@@ -107,6 +107,12 @@ public class ProviderAPI extends ProviderApiBase {
else
provider_dot_json_string = downloadWithCommercialCA(provider_main_url + "/provider.json", provider_ca_cert_fingerprint);
+ if (!isValidJson(provider_dot_json_string)) {
+ result.putString(ERRORS, getString(malformed_url));
+ result.putBoolean(RESULT_KEY, false);
+ return result;
+ }
+
try {
JSONObject provider_json = new JSONObject(provider_dot_json_string);
provider_api_url = provider_json.getString(Provider.API_URL) + "/" + provider_json.getString(Provider.API_VERSION);
@@ -147,7 +153,7 @@ public class ProviderAPI extends ProviderApiBase {
preferences.edit().putString(Constants.KEY, eip_service_json.toString()).commit();
result.putBoolean(RESULT_KEY, true);
- } catch (JSONException e) {
+ } catch (NullPointerException | JSONException e) {
String reason_to_fail = pickErrorMessage(eip_service_json_string);
result.putString(ERRORS, reason_to_fail);
result.putBoolean(RESULT_KEY, false);
@@ -171,7 +177,7 @@ public class ProviderAPI extends ProviderApiBase {
String cert_string = downloadWithProviderCA(new_cert_string_url.toString());
- if (cert_string.isEmpty() || ConfigHelper.checkErroneousDownload(cert_string))
+ if (cert_string == null || cert_string.isEmpty() || ConfigHelper.checkErroneousDownload(cert_string))
return false;
else
return loadCertificate(cert_string);
@@ -253,7 +259,7 @@ public class ProviderAPI extends ProviderApiBase {
responseString = sendGetStringToServer(string_url, headerArgs, okHttpClient);
- if (responseString.contains(ERRORS)) {
+ if (responseString != null && responseString.contains(ERRORS)) {
try {
// try to download with provider CA on certificate error
JSONObject responseErrorJson = new JSONObject(responseString);