Update Encryption.php to fix issue #34599 "encrypted remote storage"

[martink-p]

Description

The details of my change are described in #34599. However, in principle it adds a wrapper function to "stream_read" which reads (and checks) until the required block size is available or there is no remaining data.

Related Issue

• Fixes stream_read not returning the requested length for encrypted remote storage #34599

Motivation and Context

This pull request fixes an issue with encrypted external WebDAV storage and was verified as a soluition with nextcloud issue nextcloud/server#9792

How Has This Been Tested?

• test environment: running owncloud docker system with external WebDAV storage at Strato HiDrive
• test case 1: several files. uploaded, checked encryption, downloaded, checked decryption.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:

[micbar]

@martink-p
Could you get CI green?

[martink-p]

@micbar
Could you please explain to me what you mean by "CI green"?

[micbar]

@martink-p
Hi Martin, sorry for the short info :-)

I am referencing to the Continuous integration tests on your PR.
Please change the Code to match owncloud Code Style.
https://doc.owncloud.com/server/developer_manual/general/codingguidelines.html

1) lib/private/Files/Stream/Encryption.php (no_spaces_inside_parenthesis, class_definition, no_trailing_whitespace, native_function_invocation, braces)
      ---------- begin diff ----------
--- Original
+++ New
@@ -314,13 +314,13 @@
 
 		do {
 			$chunk = parent::stream_read($remaining);
-			$chunk_len = strlen($chunk);
+			$chunk_len = \strlen($chunk);
 			$data .= $chunk;
 			$remaining -= $chunk_len;
-		} while ( ($remaining > 0) && ($chunk_len > 0) );
+		} while (($remaining > 0) && ($chunk_len > 0));
 
 		return $data;
-	}	
+	}
 
 	public function stream_write($data) {
 		$length = 0;

      ----------- end diff -----------

[phil-davis]

@martink-p in your local branch you can:

make test-php-style-fix

and it should make the code style changes automatically.
Then commit and push the changes.

[martink-p]

Okay. I've fixed the style issues. However, CI drone is still complaining about two aborted tests.

[sharidas]

restarted the CI.

[mmattel]

@sharidas @phil-davis seems that you need to trigger CI again (build was killed) 😄

[codecov]

Codecov Report

Merging #36179 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master   #36179      +/-   ##
============================================
+ Coverage     64.86%   64.87%   +<.01%     
- Complexity    19787    19790       +3     
============================================
  Files          1271     1271              
  Lines         74728    74737       +9     
  Branches       1309     1309              
============================================
+ Hits          48475    48484       +9     
  Misses        25867    25867              
  Partials        386      386

Flag	Coverage Δ	Complexity Δ
#javascript	54% <ø> (ø)	0 <ø> (ø)	⬇️
#phpunit	66.06% <100%> (ø)	19790 <3> (+3)	⬆️

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Files/Stream/Encryption.php	94.53% <100%> (+0.28%)	56 <3> (+3)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f0981a5...0a403ea. Read the comment docs.

[martink-p]

I've clarified the purpose of my change a bit (comment in code) and commited it to trigger CI again ;)

[phil-davis]

@martink-p you need to rebase to the current master. Your branch is currently off an older point in master, and that is running some "samba" Windows share tests that fail.

[martink-p]

Okay guys...
I've rebased the commit to the actual master. However, the CI checks are still failing at some point.
I'm done with changing my commit over and over again.
Integrate my suggestions, or leave them as they are.

[mmattel]

@phil-davis @micbar CI is green now 👍
Could you review and and merge in case you are ok with?

[phil-davis]

Just typos in comments.

[phil-davis]

@micbar this needs review from someone who could comment more technically.
I could run the acceptance test suite with this core branch and encryption enabled (it is easy to do in CI from the encryption app)

[micbar]

@martink-p @phil-davis LGTM.

[micbar]

@martink-p Just fix the comments.

[phil-davis]

I added the QA labels. ToDo: review where this is called and do either or both of:
a) run the acceptance test suites against this branch with encryption enabled.
b) add extra test scenario(s) to improve coverage (if needed)

[phil-davis]

@martink-p the code from this has been merged in PR #36546
I made sure that the commit still has you as author.
Thanks for sorting this out. The change will be in release 10.4.0