Commit d71f7e7f authored by Marius Volkhart's avatar Marius Volkhart Committed by Lucio Maciel

Fix file uploading when no filters are applied

    When no file filters are specified in the server whitelist, the Android system is incorrectly given a mime type filter list including a single empty ("") mime type. Naturally, nothing matches this, and no documents can be selected for upload.

    To remedy this, the application is changed to return a null filter array if filters are not to be applied.
parent 4c7cd7ac
...@@ -58,6 +58,9 @@ jobs: ...@@ -58,6 +58,9 @@ jobs:
- run: - run:
name: Run Unit test name: Run Unit test
command: ./gradlew test command: ./gradlew test
- run:
name: Compile Instrumentation test
command: ./gradlew assembleAndroidTest
- store_artifacts: - store_artifacts:
path: app/build/reports/ path: app/build/reports/
destination: reports destination: reports
......
...@@ -114,9 +114,9 @@ dependencies { ...@@ -114,9 +114,9 @@ dependencies {
} }
testImplementation libraries.junit testImplementation libraries.junit
androidTestImplementation(libraries.expressoCore, { testImplementation libraries.truth
exclude group: 'com.android.support', module: 'support-annotations' androidTestImplementation libraries.espressoCore
}) androidTestImplementation libraries.espressoIntents
} }
kotlin { kotlin {
......
package chat.rocket.android.chatroom.ui
import android.content.Intent
import android.support.test.espresso.intent.rule.IntentsTestRule
import android.support.test.filters.LargeTest
import org.junit.Rule
import org.junit.Test
import android.app.Activity
import android.app.Instrumentation.ActivityResult
import android.support.test.InstrumentationRegistry
import android.support.test.espresso.intent.Intents.intended
import android.support.test.espresso.intent.Intents.intending
import android.support.test.espresso.intent.matcher.IntentMatchers.*
import org.hamcrest.Matchers.allOf
import org.hamcrest.Matchers.not
import org.junit.Before
@LargeTest
class ChatRoomFragmentTest {
@JvmField
@Rule
val activityRule = IntentsTestRule<ChatRoomActivity>(ChatRoomActivity::class.java, false, false)
@Before
fun stubAllExternalIntents() {
val activityIntent = InstrumentationRegistry.getTargetContext().chatRoomIntent("id", "name", "type", false, 0L)
activityRule.launchActivity(activityIntent)
intending(not(isInternal())).respondWith(ActivityResult(Activity.RESULT_OK, null))
}
@Test
fun showFileSelection_nonNullFiltersAreApplied() {
val fragment = activityRule.activity.supportFragmentManager.findFragmentByTag(ChatRoomActivity.TAG_CHAT_ROOM_FRAGMENT) as ChatRoomFragment
val filters = arrayOf("image/*")
fragment.showFileSelection(filters)
intended(allOf(
hasAction(Intent.ACTION_GET_CONTENT),
hasType("*/*"),
hasCategories(setOf(Intent.CATEGORY_OPENABLE)),
hasExtra(Intent.EXTRA_MIME_TYPES, filters)))
}
@Test
fun showFileSelection_nullFiltersAreNotApplied() {
val fragment = activityRule.activity.supportFragmentManager.findFragmentByTag(ChatRoomActivity.TAG_CHAT_ROOM_FRAGMENT) as ChatRoomFragment
fragment.showFileSelection(null)
intended(allOf(
hasAction(Intent.ACTION_GET_CONTENT),
hasType("*/*"),
hasCategories(setOf(Intent.CATEGORY_OPENABLE)),
not(hasExtraWithKey(Intent.EXTRA_MIME_TYPES))))
}
}
\ No newline at end of file
...@@ -28,7 +28,7 @@ interface ChatRoomView : LoadingView, MessageView { ...@@ -28,7 +28,7 @@ interface ChatRoomView : LoadingView, MessageView {
/** /**
* Perform file selection with the mime type [filter] * Perform file selection with the mime type [filter]
*/ */
fun showFileSelection(filter: Array<String>) fun showFileSelection(filter: Array<String>?)
/** /**
* Uploads a file to a chat room. * Uploads a file to a chat room.
......
...@@ -93,8 +93,8 @@ class ChatRoomActivity : AppCompatActivity(), HasSupportFragmentInjector { ...@@ -93,8 +93,8 @@ class ChatRoomActivity : AppCompatActivity(), HasSupportFragmentInjector {
isChatRoomSubscribed = intent.getBooleanExtra(INTENT_CHAT_IS_SUBSCRIBED, true) isChatRoomSubscribed = intent.getBooleanExtra(INTENT_CHAT_IS_SUBSCRIBED, true)
if (supportFragmentManager.findFragmentByTag("ChatRoomFragment") == null) { if (supportFragmentManager.findFragmentByTag(TAG_CHAT_ROOM_FRAGMENT) == null) {
addFragment("ChatRoomFragment", R.id.fragment_container) { addFragment(TAG_CHAT_ROOM_FRAGMENT, R.id.fragment_container) {
newInstance(chatRoomId, chatRoomName, chatRoomType, isChatRoomReadOnly, chatRoomLastSeen, newInstance(chatRoomId, chatRoomName, chatRoomType, isChatRoomReadOnly, chatRoomLastSeen,
isChatRoomSubscribed) isChatRoomSubscribed)
} }
...@@ -156,4 +156,8 @@ class ChatRoomActivity : AppCompatActivity(), HasSupportFragmentInjector { ...@@ -156,4 +156,8 @@ class ChatRoomActivity : AppCompatActivity(), HasSupportFragmentInjector {
super.onBackPressed() super.onBackPressed()
overridePendingTransition(R.anim.close_enter, R.anim.close_exit) overridePendingTransition(R.anim.close_enter, R.anim.close_exit)
} }
companion object {
const val TAG_CHAT_ROOM_FRAGMENT = "ChatRoomFragment"
}
} }
\ No newline at end of file
...@@ -479,12 +479,18 @@ class ChatRoomFragment : Fragment(), ChatRoomView, EmojiKeyboardListener, EmojiR ...@@ -479,12 +479,18 @@ class ChatRoomFragment : Fragment(), ChatRoomView, EmojiKeyboardListener, EmojiR
button_add_reaction.tag = drawableId button_add_reaction.tag = drawableId
} }
override fun showFileSelection(filter: Array<String>) { override fun showFileSelection(filter: Array<String>?) {
ui { ui {
val intent = Intent(Intent.ACTION_GET_CONTENT) val intent = Intent(Intent.ACTION_GET_CONTENT)
// Must set a type otherwise the intent won't resolve
intent.type = "*/*" intent.type = "*/*"
intent.putExtra(Intent.EXTRA_MIME_TYPES, filter)
intent.addCategory(Intent.CATEGORY_OPENABLE) intent.addCategory(Intent.CATEGORY_OPENABLE)
// Filter selectable files to those that match the whitelist for this particular server
if (filter != null) {
intent.putExtra(Intent.EXTRA_MIME_TYPES, filter)
}
startActivityForResult(intent, REQUEST_CODE_FOR_PERFORM_SAF) startActivityForResult(intent, REQUEST_CODE_FOR_PERFORM_SAF)
} }
} }
......
...@@ -84,13 +84,12 @@ fun PublicSettings.allowedMessagePinning(): Boolean = this[ALLOW_MESSAGE_PINNING ...@@ -84,13 +84,12 @@ fun PublicSettings.allowedMessagePinning(): Boolean = this[ALLOW_MESSAGE_PINNING
fun PublicSettings.allowedMessageEditing(): Boolean = this[ALLOW_MESSAGE_EDITING]?.value == true fun PublicSettings.allowedMessageEditing(): Boolean = this[ALLOW_MESSAGE_EDITING]?.value == true
fun PublicSettings.allowedMessageDeleting(): Boolean = this[ALLOW_MESSAGE_DELETING]?.value == true fun PublicSettings.allowedMessageDeleting(): Boolean = this[ALLOW_MESSAGE_DELETING]?.value == true
fun PublicSettings.uploadMimeTypeFilter(): Array<String> { fun PublicSettings.uploadMimeTypeFilter(): Array<String>? {
val values = this[UPLOAD_WHITELIST_MIMETYPES]?.value val values = this[UPLOAD_WHITELIST_MIMETYPES]?.value as String?
values?.let { it as String }?.split(",")?.let { if (!values.isNullOrBlank()) {
return it.mapToTypedArray { it.trim() } return values!!.split(",").mapToTypedArray { it.trim() }
} }
return null
return arrayOf("*/*")
} }
fun PublicSettings.uploadMaxFileSize(): Int { fun PublicSettings.uploadMaxFileSize(): Int {
......
package chat.rocket.android.server.domain
import chat.rocket.core.model.Value
import com.google.common.truth.Truth.assertThat
import org.junit.Test
class SettingsRepositoryTest {
@Test
fun `uploadMimeFilter returns null if not specified`() {
val settings = emptyMap<String, Value<Any>>()
val filter = settings.uploadMimeTypeFilter()
assertThat(filter).isNull()
}
}
\ No newline at end of file
package chat.rocket.android.server.domain
import chat.rocket.core.model.Value
import com.google.common.truth.Truth.assertThat
import org.junit.Test
import org.junit.runner.RunWith
import org.junit.runners.Parameterized
@RunWith(Parameterized::class)
class `SettingsRepository UploadMimeTypeFilter WhitelistIsSet Test`(private val allowedMimeTypes: String,
private val expectedFilter: Array<String>?) {
companion object {
@JvmStatic
@Parameterized.Parameters(name = "\"{0}\"")
fun data(): Collection<Array<Any?>> = listOf(
arrayOf<Any?>("", null),
arrayOf<Any?>(" ", null),
arrayOf<Any?>("image/*", arrayOf("image/*")),
arrayOf<Any?>("image/*,video/*", arrayOf("image/*", "video/*")),
arrayOf<Any?>("image/*, video/*", arrayOf("image/*", "video/*")),
arrayOf<Any?>("image/*,\tvideo/*", arrayOf("image/*", "video/*"))
)
}
@Test
fun test() {
val settings = mapOf<String, Value<Any>>(Pair(UPLOAD_WHITELIST_MIMETYPES, Value(allowedMimeTypes)))
val filter = settings.uploadMimeTypeFilter()
assertThat(filter).isEqualTo(expectedFilter)
}
}
...@@ -9,7 +9,7 @@ ext { ...@@ -9,7 +9,7 @@ ext {
dokka : '0.9.16', dokka : '0.9.16',
// Main dependencies // Main dependencies
support : '27.1.0', support : '27.1.1',
constraintLayout : '1.1.0', constraintLayout : '1.1.0',
androidKtx : '0.3', androidKtx : '0.3',
dagger : '2.14.1', dagger : '2.14.1',
...@@ -35,7 +35,7 @@ ext { ...@@ -35,7 +35,7 @@ ext {
// For testing // For testing
junit : '4.12', junit : '4.12',
truth : '0.36', truth : '0.36',
expresso : '3.0.1', espresso : '3.0.2',
mockito : '2.10.0' mockito : '2.10.0'
] ]
libraries = [ libraries = [
...@@ -98,7 +98,8 @@ ext { ...@@ -98,7 +98,8 @@ ext {
// For testing // For testing
junit : "junit:junit:$versions.junit", junit : "junit:junit:$versions.junit",
expressoCore : "com.android.support.test.espresso:espresso-core:${versions.expresso}", espressoCore : "com.android.support.test.espresso:espresso-core:${versions.espresso}",
espressoIntents : "com.android.support.test.espresso:espresso-intents:${versions.espresso}",
roomTest : "android.arch.persistence.room:testing:${versions.room}", roomTest : "android.arch.persistence.room:testing:${versions.room}",
truth : "com.google.truth:truth:$versions.truth", truth : "com.google.truth:truth:$versions.truth",
] ]
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment