fix(chunk-file-request): array index out of bound exception

[alperozturk96]

Issue

ChunkFromFileChannelRequestEntity created from ChunkedFileUploadRemoteOperation via channel and chunk.

Channel and chunk values may cause the following exception thus root cause of it can be from whether loop logic in ChunkFromFileChannelRequestEntity or creation logic from ChunkedFileUploadRemoteOperation.

java.lang.ArrayIndexOutOfBoundsException:
src.length=4096, srcPos=0,
dst.length=2048, dstPos=0,
length=-1

Exception

05-08 22:10:20.370 14592 14618 E UploadFileOperation:
Upload of
/storage/emulated/0/Android/media/com.t_example.android.webdav/
com.t_example.android.webdav/tmp/120049010000000300218142@example.com/
A090125/1GB.bin
to
/A090125/1GB.bin

Unexpected exception:

java.lang.ArrayIndexOutOfBoundsException:
src.length=4096, srcPos=0,
dst.length=2048, dstPos=0,
length=-1

Stack trace:
    at java.lang.System.arraycopy(System.java:528)
    at java.io.BufferedOutputStream.write(BufferedOutputStream.java:128)
    at com.owncloud.android.lib.common.network.ChunkFromFileChannelRequestEntity.writeRequest(ChunkFromFileChannelRequestEntity.java:110)
    at org.apache.commons.httpclient.methods.EntityEnclosingMethod.writeRequestBody(EntityEnclosingMethod.java:499)
    at org.apache.commons.httpclient.HttpMethodBase.writeRequest(HttpMethodBase.java:2114)
    at org.apache.commons.httpclient.HttpMethodBase.execute(HttpMethodBase.java:1096)
    at org.apache.commons.httpclient.HttpMethodDirector.executeWithRetry(HttpMethodDirector.java:398)
    at org.apache.commons.httpclient.HttpMethodDirector.executeMethod(HttpMethodDirector.java:171)
    at org.apache.commons.httpclient.HttpClient.executeMethod(HttpClient.java:397)
    at org.apache.commons.httpclient.HttpClient.executeMethod(HttpClient.java:323)
    at com.owncloud.android.lib.common.OwnCloudClient.executeMethod(OwnCloudClient.java:192)
    at com.owncloud.android.lib.resources.files.ChunkedFileUploadRemoteOperation.uploadChunk(ChunkedFileUploadRemoteOperation.java:294)
    at com.owncloud.android.lib.resources.files.ChunkedFileUploadRemoteOperation.run(ChunkedFileUploadRemoteOperation.java:201)
    at com.owncloud.android.lib.common.operations.RemoteOperation.execute(RemoteOperation.java:193)
    at com.owncloud.android.operations.UploadFileOperation.normalUpload(UploadFileOperation.java:1162)
    at com.owncloud.android.operations.UploadFileOperation.run(UploadFileOperation.java:503)
    at com.owncloud.android.lib.common.operations.RemoteOperation.execute(RemoteOperation.java:193)
    at com.nextcloud.client.jobs.upload.FileUploadWorker$upload$2.invokeSuspend(FileUploadWorker.kt:363)
    at com.nextcloud.client.jobs.upload.FileUploadWorker$upload$2.invoke(Unknown Source:8)
    at com.nextcloud.client.jobs.upload.FileUploadWorker$upload$2.invoke(Unknown Source:4)
    at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndispatchedOrReturn(Undispatched.kt:42)
    at kotlinx.coroutines.BuildersKt__Builders_commonKt.withContext(Builders.common.kt:164)
    at kotlinx.coroutines.BuildersKt.withContext(Unknown Source:1)
    at com.nextcloud.client.jobs.upload.FileUploadWorker.upload(FileUploadWorker.kt:358)
    at com.nextcloud.client.jobs.upload.FileUploadWorker.access$upload(FileUploadWorker.kt:56)
    at com.nextcloud.client.jobs.upload.FileUploadWorker$uploadFiles$2$result$1.invokeSuspend(FileUploadWorker.kt:287)
    at com.nextcloud.client.jobs.upload.FileUploadWorker$uploadFiles$2$result$1.invoke(Unknown Source:8)
    at com.nextcloud.client.jobs.upload.FileUploadWorker$uploadFiles$2$result$1.invoke(Unknown Source:4)
    at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndispatchedOrReturn(Undispatched.kt:42)
    at kotlinx.coroutines.BuildersKt__Builders_commonKt.withContext(Builders.common.kt:164)
    at kotlinx.coroutines.BuildersKt.withContext(Unknown Source:1)

Changes

• Reverses while loop condition to better readability (remaning = max count - chunk start)
• Checks channel read and exit if invalid
• Removes iterator

[github-actions]

SpotBugs

Category	Base	New
Bad practice	31	31
Correctness	29	28
Dodgy code	14	14
Internationalization	6	6
Malicious code vulnerability	26	26
Multithreaded correctness	7	7
Performance	7	7
Total	120	119

[alperozturk96]

Why +1 added to nextByte here? Introduced with for S3 multipart upload but why +1? Because Validates the DAV chunk filename name to be numeric between 1 and 10000 ??

And here we.getContentLenght(); can be null thus nextByte is 0?

@tobiasKaminsky