From 339f8327dead8147662cfb0be1b37f9a9621aee9 Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sat, 27 Nov 2021 12:27:25 +0300 Subject: [PATCH] Implement URL input format checks --- .../mealient/data/auth/impl/AuthRepoImpl.kt | 19 +++++++++++++- .../data/auth/impl/AuthenticationError.kt | 1 + .../ui/auth/AuthenticationFragment.kt | 6 ++++- app/src/main/res/values-ru/strings.xml | 1 + app/src/main/res/values/strings.xml | 1 + .../data/auth/impl/AuthRepoImplTest.kt | 26 +++++++++++++++++++ 6 files changed, 52 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt index 85c4909..f981b92 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt @@ -1,10 +1,14 @@ package gq.kirmanak.mealient.data.auth.impl +import android.net.Uri +import androidx.annotation.VisibleForTesting import gq.kirmanak.mealient.data.auth.AuthDataSource import gq.kirmanak.mealient.data.auth.AuthRepo import gq.kirmanak.mealient.data.auth.AuthStorage +import gq.kirmanak.mealient.data.auth.impl.AuthenticationError.MalformedUrl import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map +import okhttp3.HttpUrl.Companion.toHttpUrl import timber.log.Timber import javax.inject.Inject @@ -18,7 +22,7 @@ class AuthRepoImpl @Inject constructor( baseUrl: String ) { Timber.v("authenticate() called with: username = $username, password = $password, baseUrl = $baseUrl") - val url = if (baseUrl.startsWith("http")) baseUrl else "https://$baseUrl" + val url = parseBaseUrl(baseUrl) val accessToken = dataSource.authenticate(username, password, url) Timber.d("authenticate result is $accessToken") storage.storeAuthData(accessToken, url) @@ -40,4 +44,17 @@ class AuthRepoImpl @Inject constructor( Timber.v("logout() called") storage.clearAuthData() } + + @VisibleForTesting + fun parseBaseUrl(baseUrl: String): String = try { + val withScheme = Uri.parse(baseUrl).let { + if (it.scheme == null) it.buildUpon().scheme("https").build() + else it + }.toString() + withScheme.toHttpUrl().toString() + } catch (e: Throwable) { + Timber.e(e, "authenticate: can't parse base url $baseUrl") + throw MalformedUrl(e) + } + } \ No newline at end of file diff --git a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthenticationError.kt b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthenticationError.kt index bb9c81d..740505a 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthenticationError.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthenticationError.kt @@ -4,4 +4,5 @@ sealed class AuthenticationError(cause: Throwable) : RuntimeException(cause) { class Unauthorized(cause: Throwable) : AuthenticationError(cause) class NoServerConnection(cause: Throwable) : AuthenticationError(cause) class NotMealie(cause: Throwable) : AuthenticationError(cause) + class MalformedUrl(cause: Throwable) : AuthenticationError(cause) } diff --git a/app/src/main/java/gq/kirmanak/mealient/ui/auth/AuthenticationFragment.kt b/app/src/main/java/gq/kirmanak/mealient/ui/auth/AuthenticationFragment.kt index 5e9d507..8187a9a 100644 --- a/app/src/main/java/gq/kirmanak/mealient/ui/auth/AuthenticationFragment.kt +++ b/app/src/main/java/gq/kirmanak/mealient/ui/auth/AuthenticationFragment.kt @@ -85,9 +85,13 @@ class AuthenticationFragment : Fragment() { is Unauthorized -> getString(R.string.fragment_authentication_credentials_incorrect) else -> null } - urlInputLayout.error = when (it.exceptionOrNull()) { + urlInputLayout.error = when (val exception = it.exceptionOrNull()) { is NoServerConnection -> getString(R.string.fragment_authentication_no_connection) is NotMealie -> getString(R.string.fragment_authentication_unexpected_response) + is MalformedUrl -> { + val cause = exception.cause?.message ?: exception.message + getString(R.string.fragment_authentication_url_invalid, cause) + } is Unauthorized, null -> null else -> getString(R.string.fragment_authentication_unknown_error) } diff --git a/app/src/main/res/values-ru/strings.xml b/app/src/main/res/values-ru/strings.xml index 211f70a..89b7636 100644 --- a/app/src/main/res/values-ru/strings.xml +++ b/app/src/main/res/values-ru/strings.xml @@ -21,4 +21,5 @@ Ошибка подключения, проверьте адрес. Неожиданный ответ. Это Mealie? Что-то пошло не так, попробуйте еще раз. + Проверьте формат URL: %s \ No newline at end of file diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 40880ac..d0c8216 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -23,4 +23,5 @@ Can\'t connect, check address. Unexpected response. Is it Mealie? Something went wrong, please try again. + Check URL format: %s \ No newline at end of file diff --git a/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImplTest.kt b/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImplTest.kt index 2135f37..330518d 100644 --- a/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImplTest.kt +++ b/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImplTest.kt @@ -2,6 +2,7 @@ package gq.kirmanak.mealient.data.auth.impl import com.google.common.truth.Truth.assertThat import dagger.hilt.android.testing.HiltAndroidTest +import gq.kirmanak.mealient.data.auth.impl.AuthenticationError.MalformedUrl import gq.kirmanak.mealient.data.auth.impl.AuthenticationError.Unauthorized import gq.kirmanak.mealient.test.AuthImplTestData.TEST_PASSWORD import gq.kirmanak.mealient.test.AuthImplTestData.TEST_TOKEN @@ -50,4 +51,29 @@ class AuthRepoImplTest : MockServerTest() { subject.authenticate(TEST_USERNAME, TEST_PASSWORD, serverUrl) assertThat(subject.getBaseUrl()).isEqualTo(serverUrl) } + + @Test(expected = MalformedUrl::class) + fun `when baseUrl has ftp scheme then throws`() { + subject.parseBaseUrl("ftp://test") + } + + @Test + fun `when baseUrl scheme has one slash then corrects`() { + assertThat(subject.parseBaseUrl("https:/test")).isEqualTo("https://test/") + } + + @Test + fun `when baseUrl is single word then appends scheme and slash`() { + assertThat(subject.parseBaseUrl("test")).isEqualTo("https://test/") + } + + @Test + fun `when baseUrl is host appends scheme and slash`() { + assertThat(subject.parseBaseUrl("google.com")).isEqualTo("https://google.com/") + } + + @Test + fun `when baseUrl is correct then doesn't change`() { + assertThat(subject.parseBaseUrl("https://google.com/")).isEqualTo("https://google.com/") + } }