Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Unverified Commit 126b7428 authored by Ricki Hirner's avatar Ricki Hirner Committed by GitHub
Browse files

Decode `data` URIs of vCard 3 `PHOTO`s (#1921)

* Rename ResourceDownloader to ResourceRetriever (because it should support `data` URLs that don't have to be downloaded)

* Update `ResourceRetriever` to handle data URIs and HTTP/HTTPS URLs

* Handle invalid data URIs
parent 2de7e09c
Loading
Loading
Loading
Loading
+36 −9
Original line number Diff line number Diff line
@@ -5,7 +5,6 @@
package at.bitfire.davdroid.sync

import android.accounts.Account
import at.bitfire.dav4jvm.HttpUtils.toKtorUrl
import at.bitfire.davdroid.settings.AccountSettings
import at.bitfire.davdroid.settings.Credentials
import at.bitfire.davdroid.sync.account.TestAccount
@@ -27,7 +26,7 @@ import java.net.InetAddress
import javax.inject.Inject

@HiltAndroidTest
class ResourceDownloaderTest {
class ResourceRetrieverTest {

    @get:Rule
    val hiltRule = HiltAndroidRule(this)
@@ -36,7 +35,7 @@ class ResourceDownloaderTest {
    lateinit var accountSettingsFactory: AccountSettings.Factory

    @Inject
    lateinit var resourceDownloaderFactory: ResourceDownloader.Factory
    lateinit var resourceRetrieverFactory: ResourceRetriever.Factory

    lateinit var account: Account
    lateinit var server: MockWebServer
@@ -63,7 +62,21 @@ class ResourceDownloaderTest {


    @Test
    fun testDownload_ExternalDomain() = runTest {
    fun testRetrieve_DataUri() = runTest {
        val downloader = resourceRetrieverFactory.create(account, "example.com")
        val result = downloader.retrieve("data:image/png;base64,dGVzdA==")
        assertArrayEquals("test".toByteArray(), result)
    }

    @Test
    fun testRetrieve_DataUri_Invalid() = runTest {
        val downloader = resourceRetrieverFactory.create(account, "example.com")
        val result = downloader.retrieve("data:;INVALID,INVALID")
        assertNull(result)
    }

    @Test
    fun testRetrieve_ExternalDomain() = runTest {
        val baseUrl = server.url("/")
        val localhostIp = InetAddress.getByName(baseUrl.host).hostAddress!!

@@ -76,8 +89,8 @@ class ResourceDownloaderTest {
            .setResponseCode(200)
            .setBody("TEST"))

        val downloader = resourceDownloaderFactory.create(account, baseUrl.host)
        val result = downloader.download(baseUrlIp.toKtorUrl())
        val downloader = resourceRetrieverFactory.create(account, baseUrl.host)
        val result = downloader.retrieve(baseUrlIp.toString())

        // authentication was NOT sent because request is not for original domain
        val sentAuth = server.takeRequest().getHeader(HttpHeaders.Authorization)
@@ -88,14 +101,28 @@ class ResourceDownloaderTest {
    }

    @Test
    fun testDownload_SameDomain() = runTest {
    fun testRetrieve_FtpUrl() = runTest {
        val downloader = resourceRetrieverFactory.create(account, "example.com")
        val result = downloader.retrieve("ftp://example.com/photo.jpg")
        assertNull(result)
    }

    @Test
    fun testRetrieve_RelativeHttpsUrl() = runTest {
        val downloader = resourceRetrieverFactory.create(account, "example.com")
        val result = downloader.retrieve("https:photo.jpg")
        assertNull(result)
    }

    @Test
    fun testRetrieve_SameDomain() = runTest {
        server.enqueue(MockResponse()
            .setResponseCode(200)
            .setBody("TEST"))

        val baseUrl = server.url("/")
        val downloader = resourceDownloaderFactory.create(account, baseUrl.host)
        val result = downloader.download(baseUrl.toKtorUrl())
        val downloader = resourceRetrieverFactory.create(account, baseUrl.host)
        val result = downloader.retrieve(baseUrl.toString())

        // authentication was sent
        val sentAuth = server.takeRequest().getHeader(HttpHeaders.Authorization)
+4 −6
Original line number Diff line number Diff line
@@ -7,7 +7,6 @@ package at.bitfire.davdroid.sync
import android.accounts.Account
import android.content.ContentProviderClient
import android.text.format.Formatter
import at.bitfire.dav4jvm.ktor.toUrlOrNull
import at.bitfire.dav4jvm.okhttp.DavAddressBook
import at.bitfire.dav4jvm.okhttp.MultiResponseCallback
import at.bitfire.dav4jvm.okhttp.Response
@@ -108,7 +107,7 @@ class ContactsSyncManager @AssistedInject constructor(
    @Assisted val syncFrameworkUpload: Boolean,
    val dirtyVerifier: Optional<ContactDirtyVerifier>,
    accountSettingsFactory: AccountSettings.Factory,
    private val resourceDownloaderFactory: ResourceDownloader.Factory,
    private val resourceRetrieverFactory: ResourceRetriever.Factory,
    @SyncDispatcher syncDispatcher: CoroutineDispatcher
): SyncManager<LocalAddress, LocalAddressBook, DavAddressBook>(
    account,
@@ -368,11 +367,10 @@ class ContactsSyncManager @AssistedInject constructor(
                            jCard = isJCard,
                            downloader = object : Contact.Downloader {
                                override fun download(url: String, accepts: String): ByteArray? {
                                    // download external resource (like a photo) from an URL
                                    val httpUrl = url.toUrlOrNull() ?: return null
                                    val downloader = resourceDownloaderFactory.create(account, davCollection.location.host)
                                    // retrieve external resource (like a photo) from an URL (not necessarily HTTP[S])
                                    return runBlocking(syncDispatcher) {
                                        downloader.download(httpUrl)
                                        val retriever = resourceRetrieverFactory.create(account, davCollection.location.host)
                                        retriever.retrieve(url)
                                    }
                                }
                            }
+93 −0
Original line number Diff line number Diff line
@@ -6,14 +6,14 @@ package at.bitfire.davdroid.sync

import android.accounts.Account
import at.bitfire.davdroid.network.HttpClientBuilder
import at.bitfire.davdroid.util.DavUtils.toURIorNull
import dagger.assisted.Assisted
import dagger.assisted.AssistedFactory
import dagger.assisted.AssistedInject
import ezvcard.util.DataUri
import io.ktor.client.request.get
import io.ktor.client.statement.bodyAsBytes
import io.ktor.http.Url
import io.ktor.http.isSuccess
import java.io.IOException
import java.util.logging.Level
import java.util.logging.Logger
import javax.inject.Provider
@@ -22,15 +22,15 @@ import javax.inject.Provider
 * Downloads a separate resource that is referenced during synchronization, for instance in
 * a vCard with `PHOTO:<external URL>`.
 *
 * The [ResourceDownloader] only sends authentication for URLs on the same domain as the
 * The [ResourceRetriever] only sends authentication for URLs on the same domain as the
 * original URL. For instance, if the vCard that references a photo is taken from
 * `example.com` ([originalHost]), then [download] will send authentication
 * `example.com` ([originalHost]), then [retrieve] will send authentication
 * when downloading `https://example.com/photo.jpg`, but not for `https://external-hoster.com/photo.jpg`.
 *
 * @param account       account to build authentication from
 * @param originalHost  client only authenticates for the domain of this host
 */
class ResourceDownloader @AssistedInject constructor(
class ResourceRetriever @AssistedInject constructor(
    @Assisted private val account: Account,
    @Assisted private val originalHost: String,
    private val httpClientBuilder: Provider<HttpClientBuilder>,
@@ -39,36 +39,55 @@ class ResourceDownloader @AssistedInject constructor(

    @AssistedFactory
    interface Factory {
        fun create(account: Account, originalHost: String): ResourceDownloader
        fun create(account: Account, originalHost: String): ResourceRetriever
    }

    /**
     * Downloads the given resource and returns it as an in-memory blob.
     * Retrieves the given resource and returns it as an in-memory blob.
     * Supports HTTP/HTTPS (→ will download) and data (→ will decode) URLs.
     *
     * Authentication is handled as described in [ResourceDownloader].
     * Authentication is handled as described in [ResourceRetriever].
     *
     * @param url       URL of the resource to download
     * @param url       URL of the resource to download (`http`, `https` or `data` scheme)
     *
     * @return blob of requested resource, or `null` on error
     * @return blob of requested resource, or `null` on error or when the URL scheme is not supported
     */
    suspend fun download(url: Url): ByteArray? {
    suspend fun retrieve(url: String): ByteArray? =
        try {
            when (url.toURIorNull()?.scheme?.lowercase()) {
                "data" ->
                    DataUri.parse(url).data     // may throw IllegalArgumentException

                "http", "https" ->
                    download(url)                   // may throw various exceptions

                else ->
                    null
            }
        } catch (e: Exception) {
            logger.log(Level.SEVERE, "Couldn't retrieve resource", e)
            null
        }

    /**
     * Downloads the resource from the given HTTP/HTTPS URL.
     *
     * Doesn't catch any exceptions!
     */
    private suspend fun download(url: String): ByteArray? =
        httpClientBuilder
            .get()
            .fromAccount(account, authDomain = originalHost)  // restricts authentication to original domain
            .followRedirects(true)      // allow redirects
            .buildKtor()
            .use { httpClient ->
                try {
                val response = httpClient.get(url)
                if (response.status.isSuccess())
                    return response.bodyAsBytes()
                    else
                else {
                    logger.warning("Couldn't download external resource (${response.status})")
                } catch(e: IOException) {
                    logger.log(Level.SEVERE, "Couldn't download external resource", e)
                }
                    null
                }
        return null
            }

}
 No newline at end of file