Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Closes #10157 Don't allow data URLs to be stored in the downloads db
Browse files Browse the repository at this point in the history
  • Loading branch information
Amejia481 authored and mergify[bot] committed Apr 27, 2021
1 parent dec1bf4 commit 4219114
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,49 @@ class OnDeviceDownloadStorageTest {
assertTrue(DownloadStorage.isSameDownload(download3, downloads[2]!!))
}

@Test
fun testAddingDataURLDownload() = runBlockingTest {
val download1 = createMockDownload("1", "data:text/plain;base64,SGVsbG8sIFdvcmxkIQ==")
val download2 = createMockDownload("2", "url2")

storage.add(download1)
storage.add(download2)

val downloads = getDownloadsPagedList()

assertEquals(2, downloads.size)

assertTrue(DownloadStorage.isSameDownload(download1.copy(url = ""), downloads.first()))
assertTrue(DownloadStorage.isSameDownload(download2, downloads[1]!!))
}

@Test
fun testUpdatingDataURLDownload() = runBlockingTest {
val download1 = createMockDownload("1", "url1")
val download2 = createMockDownload("2", "url2")

storage.add(download1)
storage.add(download2)

var downloads = getDownloadsPagedList()

assertEquals(2, downloads.size)

assertTrue(DownloadStorage.isSameDownload(download1, downloads.first()))
assertTrue(DownloadStorage.isSameDownload(download2, downloads[1]!!))

val updatedDownload1 = createMockDownload("1", "data:text/plain;base64,SGVsbG8sIFdvcmxkIQ==")
val updatedDownload2 = createMockDownload("2", "updated_url2")

storage.update(updatedDownload1)
storage.update(updatedDownload2)

downloads = getDownloadsPagedList()

assertTrue(DownloadStorage.isSameDownload(updatedDownload1.copy(url = ""), downloads.first()))
assertTrue(DownloadStorage.isSameDownload(updatedDownload2, downloads[1]!!))
}

@Test
fun testRemovingDownload() = runBlockingTest {
val download1 = createMockDownload("1", "url1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,22 @@ internal data class DownloadEntity(
}

internal fun DownloadState.toDownloadEntity(): DownloadEntity {

/**
* Data URLs cause problems when restoring the values from the db,
* as the string could be so long that it could break the maximum allowed size for a cursor,
* causing SQLiteBIobTooBigException when restoring downloads from the DB.
*/
val isDataURL = url.startsWith("data:")
val sanitizedURL = if (isDataURL) {
""
} else {
url
}

return DownloadEntity(
id,
url,
sanitizedURL,
fileName,
contentType,
contentLength,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import android.os.Environment
import androidx.test.ext.junit.runners.AndroidJUnit4
import mozilla.components.browser.state.state.content.DownloadState
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith

Expand Down Expand Up @@ -64,4 +65,30 @@ class DownloadEntityTest {
assertEquals(downloadState.destinationDirectory, downloadEntity.destinationDirectory)
assertEquals(downloadState.createdTime, downloadEntity.createdAt)
}

@Test
fun `GIVEN a download with data URL WHEN converting a DownloadState to DownloadEntity THEN data url is removed`() {
val downloadState = DownloadState(
id = "1",
url = "data:text/plain;base64,SGVsbG8sIFdvcmxkIQ=="
)

val downloadEntity = downloadState.toDownloadEntity()

assertEquals(downloadState.id, downloadEntity.id)
assertTrue(downloadEntity.url.isEmpty())
}

@Test
fun `GIVEN a download with no data URL WHEN converting a DownloadState to DownloadEntity THEN data url is not removed`() {
val downloadState = DownloadState(
id = "1",
url = "url"
)

val downloadEntity = downloadState.toDownloadEntity()

assertEquals(downloadState.id, downloadEntity.id)
assertEquals(downloadState.url, downloadEntity.url)
}
}
1 change: 1 addition & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ permalink: /changelog/
* **feature-downloads**:
* ⚠️ **This is a breaking change**: `AbstractFetchDownloadService.openFile()` changed its signature from `AbstractFetchDownloadService.openFile(context: Context, filePath: String, contentType: String?)` to `AbstractFetchDownloadService.openFile(applicationContext: Context, download: DownloadState)`.
* 🚒 Bug fixed [issue #](https://github.com/mozilla-mobile/android-components/issues/10138) - The downloaded files cannot be seen.
* 🚒 Bug fixed [issue #10157](https://github.com/mozilla-mobile/android-components/issues/10157) - Crash on startup when tying to restore data URLs from the db.

* **browser-engine-gecko(-nightly/beta)**
* ⚠️ From now on there will be only one `concept-engine` implementation using [GeckoView](https://mozilla.github.io/geckoview/). On `master` this will be the Nightly version. In release versions it will be the corresponding Beta or Release version. More about this in [RFC 7](https://mozac.org/rfc/0007-synchronized-releases).
Expand Down

0 comments on commit 4219114

Please sign in to comment.