Skip to content

Commit a7ae6df

Browse files
susnuxsusnux
committed
fix(files_external): adjust custom sftp stream for phpseclib v3
The implementation was using private fields of v2, which are now properly marked as private. So either drop the custom streams or use reflection. Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
1 parent a38f348 commit a7ae6df

5 files changed

Lines changed: 82 additions & 22 deletions

File tree

‎apps/files_external/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@
9696
'OCA\\Files_External\\Lib\\Storage\\OwnCloud' => $baseDir . '/../lib/Lib/Storage/OwnCloud.php',
9797
'OCA\\Files_External\\Lib\\Storage\\SFTP' => $baseDir . '/../lib/Lib/Storage/SFTP.php',
9898
'OCA\\Files_External\\Lib\\Storage\\SFTPReadStream' => $baseDir . '/../lib/Lib/Storage/SFTPReadStream.php',
99+
'OCA\\Files_External\\Lib\\Storage\\SFTPReflection' => $baseDir . '/../lib/Lib/Storage/SFTPReflection.php',
99100
'OCA\\Files_External\\Lib\\Storage\\SFTPWriteStream' => $baseDir . '/../lib/Lib/Storage/SFTPWriteStream.php',
100101
'OCA\\Files_External\\Lib\\Storage\\SMB' => $baseDir . '/../lib/Lib/Storage/SMB.php',
101102
'OCA\\Files_External\\Lib\\Storage\\StreamWrapper' => $baseDir . '/../lib/Lib/Storage/StreamWrapper.php',

‎apps/files_external/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ class ComposerStaticInitFiles_External
111111
'OCA\\Files_External\\Lib\\Storage\\OwnCloud' => __DIR__ . '/..' . '/../lib/Lib/Storage/OwnCloud.php',
112112
'OCA\\Files_External\\Lib\\Storage\\SFTP' => __DIR__ . '/..' . '/../lib/Lib/Storage/SFTP.php',
113113
'OCA\\Files_External\\Lib\\Storage\\SFTPReadStream' => __DIR__ . '/..' . '/../lib/Lib/Storage/SFTPReadStream.php',
114+
'OCA\\Files_External\\Lib\\Storage\\SFTPReflection' => __DIR__ . '/..' . '/../lib/Lib/Storage/SFTPReflection.php',
114115
'OCA\\Files_External\\Lib\\Storage\\SFTPWriteStream' => __DIR__ . '/..' . '/../lib/Lib/Storage/SFTPWriteStream.php',
115116
'OCA\\Files_External\\Lib\\Storage\\SMB' => __DIR__ . '/..' . '/../lib/Lib/Storage/SMB.php',
116117
'OCA\\Files_External\\Lib\\Storage\\StreamWrapper' => __DIR__ . '/..' . '/../lib/Lib/Storage/StreamWrapper.php',

‎apps/files_external/lib/Lib/Storage/SFTPReadStream.php‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
use phpseclib3\Net\SSH2;
1414

1515
class SFTPReadStream implements File {
16+
use SFTPReflection;
17+
1618
/** @var resource */
1719
public $context;
1820

@@ -73,27 +75,25 @@ public function stream_open($path, $mode, $options, &$opened_path) {
7375

7476
$this->loadContext('sftp');
7577

76-
if (!($this->sftp->bitmap & SSH2::MASK_LOGIN)) {
78+
if (!($this->getSftpProperty($this->sftp, 'bitmap') & SSH2::MASK_LOGIN)) {
7779
return false;
7880
}
7981

80-
$remote_file = $this->sftp->_realpath($path);
82+
$remote_file = $this->sftp->realpath($path);
8183
if ($remote_file === false) {
8284
return false;
8385
}
8486

8587
$packet = pack('Na*N2', strlen($remote_file), $remote_file, NET_SFTP_OPEN_READ, 0);
86-
if (!$this->sftp->_send_sftp_packet(NET_SFTP_OPEN, $packet)) {
87-
return false;
88-
}
88+
$this->invokeSftp($this->sftp, 'send_sftp_packet', [NET_SFTP_OPEN, $packet]);
8989

90-
$response = $this->sftp->_get_sftp_packet();
91-
switch ($this->sftp->packet_type) {
90+
$response = $this->invokeSftp($this->sftp, 'get_sftp_packet');
91+
switch ($this->getSftpProperty($this->sftp, 'packet_type')) {
9292
case NET_SFTP_HANDLE:
9393
$this->handle = substr($response, 4);
9494
break;
9595
case NET_SFTP_STATUS: // presumably SSH_FX_NO_SUCH_FILE or SSH_FX_PERMISSION_DENIED
96-
$this->sftp->_logError($response);
96+
$this->invokeSftp($this->sftp, 'logError', [$response]);
9797
return false;
9898
default:
9999
user_error('Expected SSH_FXP_HANDLE or SSH_FXP_STATUS');
@@ -152,19 +152,24 @@ public function stream_read($count) {
152152

153153
private function request_chunk(int $size) {
154154
if ($this->pendingRead) {
155-
$this->sftp->_get_sftp_packet();
155+
$this->invokeSftp($this->sftp, 'get_sftp_packet');
156156
}
157157

158158
$packet = pack('Na*N3', strlen($this->handle), $this->handle, $this->internalPosition / 4294967296, $this->internalPosition, $size);
159159
$this->pendingRead = true;
160-
return $this->sftp->_send_sftp_packet(NET_SFTP_READ, $packet);
160+
try {
161+
$this->invokeSftp($this->sftp, 'send_sftp_packet', [NET_SFTP_READ, $packet]);
162+
return true;
163+
} catch (\Throwable) {
164+
return false;
165+
}
161166
}
162167

163168
private function read_chunk() {
164169
$this->pendingRead = false;
165-
$response = $this->sftp->_get_sftp_packet();
170+
$response = $this->invokeSftp($this->sftp, 'get_sftp_packet');
166171

167-
switch ($this->sftp->packet_type) {
172+
switch ($this->getSftpProperty($this->sftp, 'packet_type')) {
168173
case NET_SFTP_DATA:
169174
$temp = substr($response, 4);
170175
$len = strlen($temp);
@@ -220,9 +225,9 @@ public function stream_eof() {
220225
public function stream_close() {
221226
// we still have a read request incoming that needs to be handled before we can close
222227
if ($this->pendingRead) {
223-
$this->sftp->_get_sftp_packet();
228+
$this->invokeSftp($this->sftp, 'get_sftp_packet');
224229
}
225-
if (!$this->sftp->_close_handle($this->handle)) {
230+
if (!$this->invokeSftp($this->sftp, 'close_handle', [$this->handle])) {
226231
return false;
227232
}
228233
return true;

‎apps/files_external/lib/Lib/Storage/SFTPReflection.php‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\Files_External\Lib\Storage;
11+
12+
use phpseclib3\Net\SFTP;
13+
14+
/**
15+
* The SFTP read/write stream wrappers implement pipelined reads and buffered
16+
* writes directly on the SFTP protocol to get better throughput than the public
17+
* SFTP::get()/put() API can offer (a single kept-open remote handle, read-ahead,
18+
* chunked writes).
19+
*
20+
* phpseclib v3 made the required low-level packet methods and a couple of
21+
* properties private/protected. To keep the performance-oriented implementation
22+
* working without vendoring or forking phpseclib, we reach into those internals
23+
* through reflection. The reflection objects are cached because they are used
24+
* once per protocol packet.
25+
*/
26+
trait SFTPReflection {
27+
/** @var array<string, \ReflectionMethod> */
28+
private static array $sftpReflectionMethods = [];
29+
/** @var array<string, \ReflectionProperty> */
30+
private static array $sftpReflectionProperties = [];
31+
32+
/**
33+
* Invoke a method that is private in phpseclib v3.
34+
*/
35+
private function invokeSftp(SFTP $sftp, string $method, array $arguments = []): mixed {
36+
self::$sftpReflectionMethods[$method] ??= new \ReflectionMethod(SFTP::class, $method);
37+
return self::$sftpReflectionMethods[$method]->invokeArgs($sftp, $arguments);
38+
}
39+
40+
/**
41+
* Read a property that is private/protected in phpseclib v3.
42+
*/
43+
private function getSftpProperty(SFTP $sftp, string $property): mixed {
44+
self::$sftpReflectionProperties[$property] ??= new \ReflectionProperty(SFTP::class, $property);
45+
return self::$sftpReflectionProperties[$property]->getValue($sftp);
46+
}
47+
}

‎apps/files_external/lib/Lib/Storage/SFTPWriteStream.php‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
use phpseclib3\Net\SSH2;
1414

1515
class SFTPWriteStream implements File {
16+
use SFTPReflection;
17+
1618
/** @var resource */
1719
public $context;
1820

@@ -70,7 +72,7 @@ public function stream_open($path, $mode, $options, &$opened_path) {
7072

7173
$this->loadContext('sftp');
7274

73-
if (!($this->sftp->bitmap & SSH2::MASK_LOGIN)) {
75+
if (!($this->getSftpProperty($this->sftp, 'bitmap') & SSH2::MASK_LOGIN)) {
7476
return false;
7577
}
7678

@@ -82,17 +84,19 @@ public function stream_open($path, $mode, $options, &$opened_path) {
8284
}
8385

8486
$packet = pack('Na*N2', strlen($remote_file), $remote_file, NET_SFTP_OPEN_WRITE | NET_SFTP_OPEN_CREATE | NET_SFTP_OPEN_TRUNCATE, 0);
85-
if (!$this->sftp->_send_sftp_packet(NET_SFTP_OPEN, $packet)) {
87+
try {
88+
$this->invokeSftp($this->sftp, 'send_sftp_packet', [NET_SFTP_OPEN, $packet]);
89+
} catch (\Throwable) {
8690
return false;
8791
}
8892

89-
$response = $this->sftp->_get_sftp_packet();
90-
switch ($this->sftp->packet_type) {
93+
$response = $this->invokeSftp($this->sftp, 'get_sftp_packet');
94+
switch ($this->getSftpProperty($this->sftp, 'packet_type')) {
9195
case NET_SFTP_HANDLE:
9296
$this->handle = substr($response, 4);
9397
break;
9498
case NET_SFTP_STATUS: // presumably SSH_FX_NO_SUCH_FILE or SSH_FX_PERMISSION_DENIED
95-
$this->sftp->_logError($response);
99+
$this->invokeSftp($this->sftp, 'logError', [$response]);
96100
return false;
97101
default:
98102
user_error('Expected SSH_FXP_HANDLE or SSH_FXP_STATUS');
@@ -157,13 +161,15 @@ public function stream_lock($operation) {
157161
public function stream_flush() {
158162
$size = strlen($this->buffer);
159163
$packet = pack('Na*N3a*', strlen($this->handle), $this->handle, $this->internalPosition / 4294967296, $this->internalPosition, $size, $this->buffer);
160-
if (!$this->sftp->_send_sftp_packet(NET_SFTP_WRITE, $packet)) {
164+
try {
165+
$this->invokeSftp($this->sftp, 'send_sftp_packet', [NET_SFTP_WRITE, $packet]);
166+
} catch (\Throwable) {
161167
return false;
162168
}
163169
$this->internalPosition += $size;
164170
$this->buffer = '';
165171

166-
return $this->sftp->_read_put_responses(1);
172+
return $this->invokeSftp($this->sftp, 'read_put_responses', [1]);
167173
}
168174

169175
#[\Override]
@@ -174,7 +180,7 @@ public function stream_eof() {
174180
#[\Override]
175181
public function stream_close() {
176182
$this->stream_flush();
177-
if (!$this->sftp->_close_handle($this->handle)) {
183+
if (!$this->invokeSftp($this->sftp, 'close_handle', [$this->handle])) {
178184
return false;
179185
}
180186
$this->sftp->touch($this->path, time(), time());

0 commit comments

Comments
 (0)