Format adler checksum header as expected by the sync client

[ishank011]

[ishank011]

@phil-davis The tests fail with

Expected: webDav checksum should be ... ADLER32:1048035a but got ... Adler32:1048035a

But the desktop client expects the header to be Adler32 (https://github.com/owncloud/client/blob/15fc9d017fcdcc4cc95728c16a2dd171d0395b85/src/common/checksums.h#L38-L42)

And reports the error The checksum header contained an unknown checksum type 'ADLER32'

[butonic]

@phil-davis there is a mismatch between the testsuite and the client expectations. The interesting part from the build is:

@skipOnStorage:ceph @skipOnStorage:scality @files_primary_s3-issue-156 @issue-ocis-reva-196 @skipOnDbOracle | 212s
-- | --
2359 | Scenario: Restore a file and check, if the content and correct checksum is now in the current file                                                                                          # /drone/src/tmp/testrunner/tests/acceptance/features/apiVersions/fileVersions.feature:116 | 212s
2360 | Given user "Alice" has uploaded file with content "AAAAABBBBBCCCCC" and checksum "MD5:45a72715acdd5019c5be30bdbb75233e" to "/davtest.txt"                                                 # ChecksumContext::userHasUploadedFileWithContentAndChecksumToUsingTheAPI() | 212s
2361 | And user "Alice" has uploaded file "filesForUpload/textfile.txt" to "/davtest.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a"                                                   # ChecksumContext::userHasUploadedFileToWithChecksumUsingTheAPI() | 213s
2362 | And the version folder of file "/davtest.txt" for user "Alice" should contain "1" element                                                                                                 # FilesVersionsContext::theVersionFolderOfFileShouldContainElements() | 213s
2363 | When user "Alice" restores version index "1" of file "/davtest.txt" using the WebDAV API                                                                                                  # FilesVersionsContext::userRestoresVersionIndexOfFile() | 213s
2364 | Then the content of file "/davtest.txt" for user "Alice" should be "AAAAABBBBBCCCCC"                                                                                                      # FeatureContext::contentOfFileForUserShouldBe() | 213s
2365 | And as user "Alice" the webdav checksum of "/davtest.txt" via propfind should match "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8 MD5:45a72715acdd5019c5be30bdbb75233e ADLER32:1ecd03df" # ChecksumContext::theWebdavChecksumOfViaPropfindShouldMatch() | 213s
2366 | Expected: webDav checksum should be SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8 MD5:45a72715acdd5019c5be30bdbb75233e ADLER32:1ecd03df but got SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8 MD5:45a72715acdd5019c5be30bdbb75233e Adler32:1ecd03df | 213s
2367 | Failed asserting that two strings are equal. | 213s
2368 | --- Expected | 213s
2369 | +++ Actual | 213s
2370 | @@ @@ | 213s
2371 | -'SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8 MD5:45a72715acdd5019c5be30bdbb75233e ADLER32:1ecd03df' | 213s
2372 | +'SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8 MD5:45a72715acdd5019c5be30bdbb75233e Adler32:1ecd03df'

@issue-ocis-reva-196 tells me there was an issue at https://github.com/owncloud/ocis-reva/issues/196, which now redirects to https://github.com/owncloud/ocis/issues/1291. Which was closed with checksums, especially Line https://github.com/cs3org/reva/pull/1400/files#diff-ee636c0aec547c9f741aea9aadac81425c9be94c4cc9affed542a01f74f96c8dR151 where I implemented the uppercase match.

The tests can be found in core: https://github.com/owncloud/core/search?q=ADLER32%3A1ecd03df

The oc10 code produces uppercase: https://github.com/owncloud/core/blob/817f54f53d64249166feaa0eb9231c50ca1ed7e0/lib/private/Files/Storage/Wrapper/Checksum.php#L42

The client uses a case insensitive match in https://github.com/owncloud/client/blob/15fc9d017fcdcc4cc95728c16a2dd171d0395b85/src/common/checksums.cpp#L153

now there are two places where we return the checksum:

1. in a propfind as <oc:checksum>MD5:iaeo SHA1:eiao ADLER32:azpa</oc:checksum>
2. in an upload response as a Checksum header, covered with https://github.com/owncloud/core/blob/e921ceb2563a717459401385a1d20fc2a22c132f/tests/acceptance/features/apiMain/checksums.feature#L37-L47 which oc10 produces in https://github.com/owncloud/core/blob/8b743700e8bb5a9e89b8f56dd209ca48460e4807/apps/dav/lib/Connector/Sabre/FilesPlugin.php#L312-L315 which always returns SHA1:<hash> because it is hardcoded to sha1 in oc10 and then uppercased in https://github.com/owncloud/core/blob/8b743700e8bb5a9e89b8f56dd209ca48460e4807/apps/dav/lib/Connector/Sabre/File.php#L719

So both places always return uppercase in oc10. AFAICT the desktop client only uses the constant checkSumAdlerC with Adler32 in the computeNow hook which is then used internally to compute a checksum ...

@dragotin @TheOneRing please help me out understanding if the client actually would pick up a ADLER32 in

1. A PROPFIND response
2. An upload response as OC-Checksum header.

[TheOneRing]

There was an issue in desktop clients < 2.7.6 fixed in https://github.com/owncloud/client/pull/8371 . The issue was first discovered in https://github.com/owncloud/client/issues/7999 .

[butonic]

@ishank011 I think we should stick to the oc10 Uppercase for all hashing algorithms. The desktop client 2.7.6 is case insensitive.

[ishank011]

@butonic We won't be able to support older clients with ocis then. @SamuAlfageme I'm not sure what the upgrade timeline looks like from our end?

If oc10 always sends the header in uppercase, how do clients older than 2.7.6 work with it? I guess you don't run tests with Adler?

[phil-davis]

We can easily change the acceptance test expectations. If all known current clients (desktop, Android, iOS etc) that do any processing of ADLER32/Adler32 can handle any upper/lower case combinations, and if there are some (or one) older clients version(s) that really look for Adler32 then a change from ADLER32 to Adler32 might be good/useful.

IMO, we should do the same in oC10 so that all implementations respond with the same Adler32 - and we will need to check various client versions to make sure that nothing breaks for a reasonable range of existing clients.

[TheOneRing]

@butonic We won't be able to support older clients with ocis then. @SamuAlfageme I'm not sure what the upgrade timeline looks like from our end?

If oc10 always sends the header in uppercase, how do clients older than 2.7.6 work with it? I guess you don't run tests with Adler?

At the moment we only support the latest releases of the Desktop client, older clients don't support ocis properly. For older distributions we offer AppImages (2.10 support is limited to ubuntu 18.04+ the next release will support centos7+)

[labkode]

We can easily change the acceptance test expectations. If all known current clients (desktop, Android, iOS etc) that do any processing of ADLER32/Adler32 can handle any upper/lower case combinations, and if there are some (or one) older clients version(s) that really look for Adler32 then a change from ADLER32 to Adler32 might be good/useful.

IMO, we should do the same in oC10 so that all implementations respond with the same Adler32 - and we will need to check various client versions to make sure that nothing breaks for a reasonable range of existing clients.

This is the key point. Clients process this attribute as case insensitive. Artificial tests (the ones failing) do not honour this and change the semantics to case sensitive, meaning that they are not testing anything useful.

Conclusion: the tests needs to be adapted to be case insensitive to ensure client behaviours are properly taken into account.

@phil-davis can we adapt the tests please to be case insensitive please?