TUS uploads might lead to data-loss in sharing conditions #4153
Comments
SwikritiT
changed the title
spaces in nightlyspaces in nightly
|
Tus upload related tests are failing in other merge PR as well seems like there's some problem Scenario Outline: Sharer uploads a file with checksum and as a sharee overwrites the shared file with new data and correct checksum # /srv/app/testrunner/tests/acceptance/features/apiWebdavUploadTUS/uploadToShare.feature:329
Given using <dav_version> DAV path # FeatureContext::usingOldOrNewDavPath()
And user "Alice" has created a new TUS resource on the WebDAV API with these headers: # TUSContext::createNewTUSResource()
| Upload-Length | 16 |
| Upload-Metadata | filename dGV4dEZpbGUudHh0 |
And user "Alice" has uploaded file with checksum "SHA1 c1dab0c0864b6ac9bdd3743a1408d679f1acd823" to the last created TUS Location with offset "0" and content "original content" using the TUS protocol on the WebDAV API # TUSContext::userHasUploadedFileWithChecksum()
And user "Alice" has shared file "/textFile.txt" with user "Brian" # FeatureContext::userHasSharedFileWithUserUsingTheSharingApi()
And user "Brian" has accepted share "/textFile.txt" offered by user "Alice" # FeatureContext::userHasReactedToShareOfferedBy()
When user "Brian" overwrites recently shared file with offset "0" and data "overwritten content" with checksum "SHA1 fe990d2686a0fc86004efc31f5bf2475a45d4905" using the TUS protocol on the WebDAV API with these headers: # TUSContext::userOverwritesFileWithChecksum()
| Upload-Length | 19 |
| Upload-Metadata | filename L1NoYXJlcy90ZXh0RmlsZS50eHQ= |
Then the HTTP status code should be "204" # FeatureContext::thenTheHTTPStatusCodeShouldBe()
And the content of file "/textFile.txt" for user "Alice" should be "overwritten content" # FeatureContext::contentOfFileForUserShouldBe()
Examples:
| dav_version |
| old |
| new |
cURL error 52: Empty reply from server (see https://curl.haxx.se/libcurl/c/libcurl-errors.html) for https://ocis-server:9200/remote.php/dav/files/Alice/textFile.txt (GuzzleHttp\Exception\ConnectException)
| spaces |
HTTP status code 412 is not the expected value 201
Failed asserting that 412 matches expected 201. |
|
Fails here too : https://drone.owncloud.com/owncloud/ocis/13192/49/6 |
|
Failed here too: https://drone.owncloud.com/owncloud/ocis/13284/51/7 |
|
Failed here too: https://drone.owncloud.com/owncloud/ocis/13309/50/7 |
|
Failed here too: https://drone.owncloud.com/owncloud/ocis/13320/50/6 |
|
The test fails for me locally every time in spaces because in this step When user "Brian" uploads file with content "overwritten content" to "/Shares/FOLDER/textfile.txt" using the TUS protocol on the WebDAV API # TUSContext::userUploadsAFileWithContentToUsingTus()the overwriting content is uploaded to We need to decide if we want to do the same for #4154 other tests too? |
|
I'm surprised that worked ever, tests that use sharing and spaces should not be in core but in ocis repo and work with the share space. So same as #4154 |
The related tests for spaces are in the expected to fail file but the tests aren't exactly doing what they are supposed to do so idk how revelent it is to put them in expected to fail ocis/tests/acceptance/expected-failures-API-on-OCIS-storage.md Lines 1044 to 1055 in f732362 The test is failing in |
|
ocis does show accepted shares in |
|
@butonic with "suboptimal" you mean slow but still should work? Tests using the share jail should be placed in ocis repo |
|
correct, slow, but should work |
|
failed on |
|
ocis log output: |
SwikritiT
changed the title
spaces in nightlynew dav
|
I could reproduce the issue locally by running the curl commands involved in the tests in the loop. And seems like the overwritten content uploaded by the share receiver is not accessible by the sharer sometimes.
response of patch
For this in some cases server replies with ocis log Upon accessing through UI the overwritten content is uploaded for the share-reciever tus.mp4 |
individual-it
changed the title
new dav
|
@butonic I really want to understand the underlying problem. Can you take a look? |
micbar
added
the
Priority:p2-high
|
👁️ |
|
weird ... I rebuild the steps in postman and let them run 100 times ... not a single error. maybe postman is toooo slow 🤔 @SwikritiT thx for the awesome detailed steps! Why are you sending a Do you have a bash script I can use to try to reproduce this? |
|
ok so the testsuite uses the checksum ... the header for that is |
|
after adding the |
|
in the ocdav get.go we have: if info.Checksum != nil {
w.Header().Set(net.HeaderOCChecksum, fmt.Sprintf("%s:%s", strings.ToUpper(string(storageprovider.GRPC2PKGXS(info.Checksum.Type))), info.Checksum.Sum))
}
var c int64
if c, err = io.Copy(w, httpRes.Body); err != nil {
log.Error().Err(err).Msg("error finishing copying data to response")
}when logging the error we have a maximally inconsistent state:
something to dig into tomorrow ... might be a race where the download / get happens tooo quickly. |
|
wat ... it no longer happens 🤯 |
|
ah, the last PATCH request was taking 164ms which seems to have prevented the glitch. aftor wiping .ocis (to get rid of ocd shares, which currently slow down the system significantly) the request took 64ms and now I am in the error case again. |
|
so get.go first does a Stat, which returns size 16, the original size, then does an InitiateFileDownload, which returns a download url that carries the correct Content-Length 19 ... might be a cache invalidation problem ? ... |
I had a script to reproduce it but seems like you've reproduced it already |
|
TODO QA team after this issue is fixed || reva is bumped on ocis
|
|
Similar failure but not in sharing condition Scenario Outline: upload a twice file in chunks with TUS and versions are available # /drone/src/oc10/testrunner/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature:172
Given using <dav_version> DAV path # FeatureContext::usingOldOrNewDavPath()
When user "Alice" uploads file with content "0123456789" in 10 chunks to "/myChunkedFile.txt" using the TUS protocol on the WebDAV API # TUSContext::userUploadsAFileWithContentInChunksUsingTus()
And user "Alice" uploads file with content "01234" in 5 chunks to "/myChunkedFile.txt" using the TUS protocol on the WebDAV API # TUSContext::userUploadsAFileWithContentInChunksUsingTus()
Then the HTTP status code should be "200" # FeatureContext::thenTheHTTPStatusCodeShouldBe()
And the version folder of file "/myChunkedFile.txt" for user "Alice" should contain "1" elements # FilesVersionsContext::theVersionFolderOfFileShouldContainElements()
And the content of file "/myChunkedFile.txt" for user "Alice" should be "01234" # FeatureContext::contentOfFileForUserShouldBe()
Examples:
| dav_version |
| old |
| new |
cURL error 52: Empty reply from server (see https://curl.haxx.se/libcurl/c/libcurl-errors.html) for https://ocis-server:9200/remote.php/dav/files/Alice/myChunkedFile.txt (GuzzleHttp\Exception\ConnectException)
| spaces | |
|
hm ... I cannot reproduce this ... this is my script: #!/bin/bash
echo "initiating first upload"
location=$(curl --location --request POST 'https://cloud.ocis.test/remote.php/dav/files/admin' -k \
--header 'Tus-Resumable: 1.0.0' \
--header 'Upload-Length: 10' \
--header 'Upload-Metadata: filename bXlDaHVua2VkRmlsZS50eHQ=' \
--header 'Authorization: Basic YWRtaW46YWRtaW4=' -I 2>|/dev/null | awk '/^location:/ { print $2 }' | sed -e 's/[[:cntrl:]]//')
echo "uploading at $location"
for i in 0 1 2 3 4 5 6 7 8 9; do
curl --request PATCH ${location} -k \
--header 'Tus-Resumable: 1.0.0' \
--header 'Content-Type: application/offset+octet-stream' \
--header "Upload-Offset: $i" \
--header 'Authorization: Basic YWRtaW46YWRtaW4=' \
--data-raw $i
done
echo "initiating second upload"
location=$(curl --location --request POST 'https://cloud.ocis.test/remote.php/dav/files/admin' -k \
--header 'Tus-Resumable: 1.0.0' \
--header 'Upload-Length: 5' \
--header 'Upload-Metadata: filename bXlDaHVua2VkRmlsZS50eHQ=' \
--header 'Authorization: Basic YWRtaW46YWRtaW4=' -I 2>|/dev/null | awk '/^location:/ { print $2 }' | sed -e 's/[[:cntrl:]]//')
echo "overwriting at $location"
for i in 0 1 2 3 4; do
curl --request PATCH ${location} -k \
--header 'Tus-Resumable: 1.0.0' \
--header 'Content-Type: application/offset+octet-stream' \
--header "Upload-Offset: $i" \
--header 'Authorization: Basic YWRtaW46YWRtaW4=' \
--data-raw $i
done
echo 'looking up fileid'
fileid=$(curl --location --request PROPFIND 'https://cloud.ocis.test/remote.php/dav/files/admin/myChunkedFile.txt' -k \
--header 'Authorization: Basic YWRtaW46YWRtaW4=' \
--header 'Content-Type: application/xml' \
--data-raw '<?xml version="1.0"?>
<d:propfind xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns">
<d:prop>
<oc:fileid />
</d:prop>
</d:propfind>' 2>|/dev/null | sed s@'.*.oc:fileid.\(.*\)..oc:fileid.*'@'\1'@)
echo 'fetching versions'
versions=$(curl --request PROPFIND "https://cloud.ocis.test/remote.php/dav/meta/${fileid}/v" -k \
--header 'Authorization: Basic YWRtaW46YWRtaW4=' -s)
echo 'downloading'
content=$(curl --request GET "https://cloud.ocis.test/remote.php/dav/files/admin/myChunkedFile.txt" -k \
--header 'Authorization: Basic YWRtaW46YWRtaW4=' -s)
echo 'deleting'
curl --request DELETE "https://cloud.ocis.test/remote.php/dav/files/admin/myChunkedFile.txt" -k \
--header 'Authorization: Basic YWRtaW46YWRtaW4='AFAICT curl does not get a reply for the download request ... maybe the ci run still has an ocis error in the logs 👀 |
|
The only place in the error level logs that actually has an error after the last chunk is uploaded looks like this: but that would be a problem for the subsequest upload ... 🤔 and the why is that line logged three times .... 🤔 |
|
hm also cannot reproduce against /dav/spaces |
|
latest failure in https://drone.owncloud.com/owncloud/ocis/13609/51/7, |
I'll make a PR to unskip these tests |
Following scenario failed in nightly and needs further investigation https://drone.owncloud.com/owncloud/ocis/13277/49/6
The text was updated successfully, but these errors were encountered: