Skip to content

Commit 4d76ea3

Browse files
IIMacGyverIIIIMacGyverII
committed
fix(upload): prevent attachment sharing failure caused by chunked upload filecache bug
Files sent as chat attachments never appeared in the conversation. Investigation showed the chunked upload MOVE returned HTTP 201 but a subsequent HEAD on the assembled file returned 404 - Nextcloud's server was not registering the file in oc_filecache on assembly. The OCS sharing API uses the filecache, so it also returned 404 'Wrong path, file/folder does not exist' despite the file existing on disk. Raise the chunked upload threshold from 1 MB to 20 MB so that typical phone photos go through direct PUT upload, which correctly updates the filecache. Chunked upload is still used for files over 20 MB. Additional fixes: - ChunkedFileUploader: build a fresh OkHttpClient without OCS-APIRequest and Accept: application/json headers, which were causing sabre/dav to return JSON 200 instead of XML 207 for PROPFIND, breaking dav4jvm. PROPFIND failures now fall back gracefully to uploading all chunks fresh. - FileUploader: fix createRequestBody() to use readBytes() instead of available(), which returns 0 for content:// URIs. - ShareOperationWorker: retry share creation up to 4 times with 2s delay on 404, as a safety net for large files using chunked upload. Signed-off-by: IIMacGyverII <macgyverjunk@yahoo.com> AI-assistant: Copilot 0.45.1 (Claude Sonnet 4.6)
1 parent a637cc1 commit 4d76ea3

4 files changed

Lines changed: 88 additions & 52 deletions

File tree

‎app/src/main/java/com/nextcloud/talk/jobs/ShareOperationWorker.kt‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_INTERNAL_USER_ID
2626
import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_META_DATA
2727
import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_ROOM_TOKEN
2828
import io.reactivex.schedulers.Schedulers
29+
import retrofit2.HttpException
2930
import javax.inject.Inject
3031

3132
@AutoInjector(NextcloudTalkApplication::class)
@@ -46,6 +47,16 @@ class ShareOperationWorker(context: Context, workerParams: WorkerParameters) : W
4647

4748
override fun doWork(): Result {
4849
for (filePath in filesArray) {
50+
tryCreateShare(filePath)
51+
}
52+
return Result.success()
53+
}
54+
55+
@Suppress("TooGenericExceptionCaught")
56+
private fun tryCreateShare(filePath: String?) {
57+
for (attempt in 1..SHARE_MAX_ATTEMPTS) {
58+
var succeeded = false
59+
var shouldRetry = false
4960
ncApi.createRemoteShare(
5061
credentials,
5162
ApiUtils.getSharingUrl(baseUrl!!),
@@ -56,11 +67,18 @@ class ShareOperationWorker(context: Context, workerParams: WorkerParameters) : W
5667
)
5768
.subscribeOn(Schedulers.io())
5869
.blockingSubscribe(
59-
{},
60-
{ e -> Log.w(TAG, "error while creating RemoteShare", e) }
70+
{ succeeded = true },
71+
{ e ->
72+
if (e is HttpException && e.code() == HTTP_NOT_FOUND && attempt < SHARE_MAX_ATTEMPTS) {
73+
shouldRetry = true
74+
} else {
75+
Log.w(TAG, "error while creating RemoteShare", e)
76+
}
77+
}
6178
)
79+
if (succeeded || !shouldRetry) return
80+
Thread.sleep(SHARE_RETRY_DELAY_MS)
6281
}
63-
return Result.success()
6482
}
6583

6684
init {
@@ -78,6 +96,9 @@ class ShareOperationWorker(context: Context, workerParams: WorkerParameters) : W
7896

7997
companion object {
8098
private val TAG = ShareOperationWorker::class.simpleName
99+
private const val HTTP_NOT_FOUND = 404
100+
private const val SHARE_MAX_ATTEMPTS = 4
101+
private const val SHARE_RETRY_DELAY_MS = 2000L
81102

82103
fun shareFile(roomToken: String?, currentUser: User, remotePath: String, metaData: String?) {
83104
val paths: MutableList<String> = ArrayList()

‎app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa
308308
private const val ROOM_TOKEN = "ROOM_TOKEN"
309309
private const val CONVERSATION_NAME = "CONVERSATION_NAME"
310310
private const val META_DATA = "META_DATA"
311-
private const val CHUNK_UPLOAD_THRESHOLD_SIZE: Long = 1024000
311+
private const val CHUNK_UPLOAD_THRESHOLD_SIZE: Long = 20 * 1024 * 1024
312312
private const val NOTIFICATION_FILE_NAME_MAX_LENGTH = 20
313313
private const val THREE_DOTS = "…"
314314
private const val HUNDRED_PERCENT = 100

‎app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt‎

Lines changed: 43 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ class ChunkedFileUploader(
122122
}
123123
}
124124

125-
@Suppress("Detekt.ComplexMethod")
125+
@Suppress("Detekt.ComplexMethod", "Detekt.ReturnCount")
126126
private fun getUploadedChunks(davResource: DavResource, uploadFolderUri: String): MutableList<Chunk> {
127127
val davResponse = DavResponse()
128128
val memberElements: MutableList<at.bitfire.dav4jvm.Response> = ArrayList()
@@ -142,9 +142,14 @@ class ChunkedFileUploader(
142142
Unit
143143
}
144144
} catch (e: IOException) {
145-
throw IOException("Error reading remote path", e)
145+
// PROPFIND on Nextcloud chunked-upload folders can return unexpected responses
146+
// (e.g. 200 instead of 207). Treat any failure as "no chunks uploaded yet" so
147+
// we fall back to a full upload rather than aborting entirely.
148+
Log.w(TAG, "PROPFIND failed — assuming no chunks on server, will upload from scratch: ${e.message}")
149+
return ArrayList()
146150
} catch (e: DavException) {
147-
throw IOException("Error reading remote path", e)
151+
Log.w(TAG, "PROPFIND failed — assuming no chunks on server, will upload from scratch: ${e.message}")
152+
return ArrayList()
148153
}
149154
for (memberElement in memberElements) {
150155
remoteFiles.add(
@@ -271,21 +276,23 @@ class ChunkedFileUploader(
271276
}
272277

273278
private fun initHttpClient(okHttpClient: OkHttpClient, currentUser: User) {
274-
val okHttpClientBuilder: OkHttpClient.Builder = okHttpClient.newBuilder()
275-
okHttpClientBuilder.followRedirects(false)
276-
okHttpClientBuilder.followSslRedirects(false)
277-
// okHttpClientBuilder.readTimeout(Duration.ofMinutes(30)) // TODO set timeout
278-
okHttpClientBuilder.protocols(listOf(Protocol.HTTP_1_1))
279-
okHttpClientBuilder.authenticator(
280-
RestModule.HttpAuthenticator(
281-
ApiUtils.getCredentials(
282-
currentUser.username,
283-
currentUser.token
284-
)!!,
285-
"Authorization"
279+
val builder = OkHttpClient.Builder()
280+
.followRedirects(false)
281+
.followSslRedirects(false)
282+
.protocols(listOf(Protocol.HTTP_1_1))
283+
.sslSocketFactory(okHttpClient.sslSocketFactory, okHttpClient.x509TrustManager!!)
284+
.hostnameVerifier(okHttpClient.hostnameVerifier)
285+
.authenticator(
286+
RestModule.HttpAuthenticator(
287+
ApiUtils.getCredentials(
288+
currentUser.username,
289+
currentUser.token
290+
)!!,
291+
"Authorization"
292+
)
286293
)
287-
)
288-
this.okHttpClientNoRedirects = okHttpClientBuilder.build()
294+
okHttpClient.proxy?.let { builder.proxy(it) }
295+
this.okHttpClientNoRedirects = builder.build()
289296
}
290297

291298
private fun assembleChunks(uploadFolderUri: String, targetPath: String) {
@@ -294,6 +301,7 @@ class ChunkedFileUploader(
294301
currentUser.userId!!,
295302
targetPath
296303
)
304+
createRemoteFolder(targetPath)
297305
val originUri = "$uploadFolderUri/.file"
298306

299307
DavResource(
@@ -304,18 +312,30 @@ class ChunkedFileUploader(
304312
true
305313
) { response: Response ->
306314
if (response.isSuccessful) {
307-
ShareOperationWorker.shareFile(
308-
roomToken,
309-
currentUser,
310-
targetPath,
311-
metaData
312-
)
315+
ShareOperationWorker.shareFile(roomToken, currentUser, targetPath, metaData)
313316
} else {
314317
throw IOException("Failed to assemble chunks. response code: " + response.code)
315318
}
316319
}
317320
}
318321

322+
private fun createRemoteFolder(targetPath: String) {
323+
val folderPath = targetPath.substringBeforeLast('/')
324+
if (folderPath.isEmpty() || folderPath == "/") return
325+
val folderUri = ApiUtils.getUrlForFileUpload(currentUser.baseUrl!!, currentUser.userId!!, folderPath)
326+
try {
327+
DavResource(okHttpClientNoRedirects!!, folderUri.toHttpUrlOrNull()!!).mkCol(
328+
xmlBody = null
329+
) { _ -> }
330+
} catch (e: HttpException) {
331+
if (e.code != METHOD_NOT_ALLOWED_CODE) {
332+
Log.w(TAG, "Unexpected error creating remote folder $folderPath: ${e.code}")
333+
}
334+
} catch (e: IOException) {
335+
Log.w(TAG, "Failed to create remote folder $folderPath", e)
336+
}
337+
}
338+
319339
fun abortUpload(onSuccess: () -> Unit) {
320340
isUploadAborted = true
321341
try {

‎app/src/main/java/com/nextcloud/talk/upload/normal/FileUploader.kt‎

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import okhttp3.RequestBody
2929
import okhttp3.Response
3030
import java.io.File
3131
import java.io.IOException
32-
import java.io.InputStream
3332

3433
class FileUploader(
3534
okHttpClient: OkHttpClient,
@@ -116,37 +115,33 @@ class FileUploader(
116115
.flatMap { upload(sourceFileUri, fileName, remotePath, metaData) }
117116

118117
@Suppress("Detekt.TooGenericExceptionCaught")
119-
private fun createRequestBody(sourceFileUri: Uri): RequestBody? {
120-
var requestBody: RequestBody? = null
118+
private fun createRequestBody(sourceFileUri: Uri): RequestBody? =
121119
try {
122-
val input: InputStream = context.contentResolver.openInputStream(sourceFileUri)!!
123-
input.use {
124-
val buf = ByteArray(input.available())
125-
while (it.read(buf) != -1) {
126-
requestBody = RequestBody.create("application/octet-stream".toMediaTypeOrNull(), buf)
127-
}
128-
}
120+
val bytes = context.contentResolver.openInputStream(sourceFileUri)!!.use { it.readBytes() }
121+
RequestBody.create("application/octet-stream".toMediaTypeOrNull(), bytes)
129122
} catch (e: Exception) {
130123
Log.e(TAG, "failed to create RequestBody for $sourceFileUri", e)
124+
null
131125
}
132-
return requestBody
133-
}
134126

135127
private fun initHttpClient(okHttpClient: OkHttpClient, currentUser: User) {
136-
val okHttpClientBuilder: OkHttpClient.Builder = okHttpClient.newBuilder()
137-
okHttpClientBuilder.followRedirects(false)
138-
okHttpClientBuilder.followSslRedirects(false)
139-
okHttpClientBuilder.protocols(listOf(Protocol.HTTP_1_1))
140-
okHttpClientBuilder.authenticator(
141-
RestModule.HttpAuthenticator(
142-
ApiUtils.getCredentials(
143-
currentUser.username,
144-
currentUser.token
145-
)!!,
146-
"Authorization"
128+
val builder = OkHttpClient.Builder()
129+
.followRedirects(false)
130+
.followSslRedirects(false)
131+
.protocols(listOf(Protocol.HTTP_1_1))
132+
.sslSocketFactory(okHttpClient.sslSocketFactory, okHttpClient.x509TrustManager!!)
133+
.hostnameVerifier(okHttpClient.hostnameVerifier)
134+
.authenticator(
135+
RestModule.HttpAuthenticator(
136+
ApiUtils.getCredentials(
137+
currentUser.username,
138+
currentUser.token
139+
)!!,
140+
"Authorization"
141+
)
147142
)
148-
)
149-
this.okHttpClientNoRedirects = okHttpClientBuilder.build()
143+
okHttpClient.proxy?.let { builder.proxy(it) }
144+
this.okHttpClientNoRedirects = builder.build()
150145
}
151146

152147
@Suppress("Detekt.ThrowsCount")

0 commit comments

Comments
 (0)