From 7c96fe373898b5851cc4c8367cc24effef50d8f7 Mon Sep 17 00:00:00 2001 From: grubdragon Date: Thu, 28 Dec 2017 15:58:51 +0530 Subject: [PATCH 1/3] Closes #1976, HttpUrlConnection now allowed to follow redirects --- .../focus/settings/ManualAddSearchEngineSettingsFragment.java | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/main/java/org/mozilla/focus/settings/ManualAddSearchEngineSettingsFragment.java b/app/src/main/java/org/mozilla/focus/settings/ManualAddSearchEngineSettingsFragment.java index 10a4459953c..8fd0a620e3f 100644 --- a/app/src/main/java/org/mozilla/focus/settings/ManualAddSearchEngineSettingsFragment.java +++ b/app/src/main/java/org/mozilla/focus/settings/ManualAddSearchEngineSettingsFragment.java @@ -219,6 +219,7 @@ private void showServerError(final ManualAddSearchEngineSettingsFragment that) { HttpURLConnection connection = null; try { connection = (HttpURLConnection) searchURL.openConnection(); + connection.setInstanceFollowRedirects(true); connection.setConnectTimeout(SEARCH_QUERY_VALIDATION_TIMEOUT_MILLIS); connection.setReadTimeout(SEARCH_QUERY_VALIDATION_TIMEOUT_MILLIS); From 902c150fe145fdf9eba7c4bbab3f1419f40c47b4 Mon Sep 17 00:00:00 2001 From: grubdragon Date: Sat, 6 Jan 2018 13:43:28 +0530 Subject: [PATCH 2/3] Changed the sanity check on the HttpURLConnection, now compares with 300 --- .../focus/settings/ManualAddSearchEngineSettingsFragment.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/mozilla/focus/settings/ManualAddSearchEngineSettingsFragment.java b/app/src/main/java/org/mozilla/focus/settings/ManualAddSearchEngineSettingsFragment.java index 8fd0a620e3f..a0779c3e127 100644 --- a/app/src/main/java/org/mozilla/focus/settings/ManualAddSearchEngineSettingsFragment.java +++ b/app/src/main/java/org/mozilla/focus/settings/ManualAddSearchEngineSettingsFragment.java @@ -223,8 +223,8 @@ private void showServerError(final ManualAddSearchEngineSettingsFragment that) { connection.setConnectTimeout(SEARCH_QUERY_VALIDATION_TIMEOUT_MILLIS); connection.setReadTimeout(SEARCH_QUERY_VALIDATION_TIMEOUT_MILLIS); - // A non-error HTTP response is good enough as a sanity check, some search engines redirect to https. - return connection.getResponseCode() < 400; + // Now that redirects are followed, 300 is a better and stronger sanity check, checks for a non error and non redirect response + return connection.getResponseCode() < 300; } catch (final IOException e) { // Don't log exception to avoid leaking URL. From 829806c8e5a2df44734274295f181eaeb726ece6 Mon Sep 17 00:00:00 2001 From: grubdragon Date: Tue, 9 Jan 2018 22:08:42 +0530 Subject: [PATCH 3/3] Changed SearchEngineValidationTest, since redirects are followed, 3xx are invalid --- .../mozilla/focus/settings/SearchEngineValidationTest.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/test/java/org/mozilla/focus/settings/SearchEngineValidationTest.kt b/app/src/test/java/org/mozilla/focus/settings/SearchEngineValidationTest.kt index ec3e9c66860..845fadada43 100644 --- a/app/src/test/java/org/mozilla/focus/settings/SearchEngineValidationTest.kt +++ b/app/src/test/java/org/mozilla/focus/settings/SearchEngineValidationTest.kt @@ -24,9 +24,9 @@ class SearchEngineValidationTest { } @Test - fun `URL using HTTP redirect is valid`() = withMockWebServer(responseWithStatus(301)) { - // Currently we accept redirects as valid. We might want to follow the redirect (Issue #1976) - assertTrue(isValidSearchQueryURL(it.rootUrl())) + fun `URL using HTTP redirect is invalid`() = withMockWebServer(responseWithStatus(301)) { + // We now follow redirects(Issue #1976). This test now asserts false. + assertFalse(isValidSearchQueryURL(it.rootUrl())) } @Test