Update manager using ssh2 doesn't error when copying doesn't work

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Rerolled the patch for Drupal 10.1.x version to resolve the issue.

Needs testing.

Updated the patch to result minor validation fixes.

@arunkumark Thanks for your re-rolls. But can you please provide an interdiff → next time? It helps to see what has changed and also to spot potential bad re-rolls :-).

Also needs work since the custom commands failed. Please check the test run as those are phpcs issues.

Oh :( I uploaded the wrong patch file. Now attached is the valid patch file or PHPCS issue fixes.

@BramDriesen thanks for the review and feedbacks.

@arunkumark BramDriesen was asking for an interdiff which wasn't provided.

Also per new policy it needs to be documented why the original patch needed a reroll.

As a bug this will need a test case

Also this will need an issue summary update to document proposed solution.

+++ b/core/lib/Drupal/Core/FileTransfer/SSH.php
@@ -55,7 +55,11 @@ protected function copyFileJailed($source, $destination) {
+    $stream = @ssh2_exec($this->connection, 'cp -Rp ' . escapeshellarg($source) . ' ' . escapeshellarg($destination) . '; echo $?');
+    stream_set_blocking($stream, TRUE);
+    $stream_out = ssh2_fetch_stream($stream, SSH2_STREAM_STDIO);
+    $status = stream_get_contents($stream_out);
+    if ($status != 0) {

I have the feeling we are casting too many variables as well. This can be converted into 2 lines of code. Also not sure if the inheritDoc is up to date for this change. This could be cleaned and simplified.

Also the != 0 check can simply be removed I think.

As per comment #18 a minor correction was made on the if statement.

Attached interdiff and updated the issue summary.

Was tagged for tests which still need to happen.

Not ready for review yet.