Bug report — v6.1.0: uploads are stored on file servers marked "Read Only" or "Disabled"
Product: Yetishare File Hosting Script Version: 6.1.0 (also present in 5.3.1) Severity: Medium to High — the server status setting is silently ignored; in some configurations files are also recorded against the wrong server, which breaks their downloads.
Summary
Two code paths assign an upload to a file server without checking the server's status. Every regular branch of the server selection correctly filters statusId = 2 (active) — these two shortcuts do not.
As a result, a server set to Read Only or Disabled in the admin area can still receive and be assigned new uploads.
Issue 1 — uploadServerOverride ignores the server status
app/helpers/FileServerHelper.class.php, getAvailableServerPoolIds(), lines 25–38:
if (($Auth->loggedIn() === true) && (int)$Auth->user->uploadServerOverride) {
$uploadServerOverride = (int) $db->getValue('SELECT uploadServerOverride '
. 'FROM users WHERE id = :id LIMIT 1', ['id' => $Auth->id]);
if ($uploadServerOverride) {
return [
$uploadServerOverride, // <-- returned without any status check
];
}
}
A user pinned to a specific server via
Edit User → Upload Server Override keeps uploading to that server after it has been set to Read Only or Disabled.
Issue 2 — the fallback in _storeFile() ignores the server status
app/services/Uploader.class.php, _storeFile() (starts line 816), lines 836–861:
// if this is a 'direct' server, and it's active, use it
$uploadServerDetails = FileHelper::getCurrentServerDetails();
if ($uploadServerDetails['serverType'] === 'direct' && (int)$uploadServerDetails['statusId'] === 2) {
$uploadServerId = $uploadServerDetails['id']; // correct: status IS checked
}
if ($uploadServerId === null) {
$uploadServerId = FileServerHelper::getAvailableServerId(false); // excludes direct servers
}
$uploadServerDetails = $db->getRow('SELECT * FROM file_server WHERE id = :id',
['id' => (int) $uploadServerId]);
if (!$uploadServerDetails) {
// if we failed to load any server, fallback on the current server
$uploadServerDetails = FileHelper::getCurrentServerDetails();
$uploadServerId = $uploadServerDetails['id']; // <-- no status check here
}
Note that the server is resolved when the upload finishes, not when it starts.
Why this fires routinely, not rarely
The pool call passes false, which adds AND serverType != "direct" to the query. On any installation whose storage is exclusively made up of direct servers — a common setup — that query can never return one of them. If the remaining non-direct servers (typically a single "Local Default") are not active, the pool returns false, the lookup on id = 0 finds nothing, and the fallback runs.
On our installation this is the permanent state: 11 direct servers plus one local server that is disabled. Every upload completing on a Read Only direct server therefore takes the fallback path and is stored there.
Two different outcomes
- Pool returns false (our case): the file is recorded against the server that physically received it. The data stays consistent, but the Read Only / Disabled setting is silently ignored — an administrator taking a server out of rotation has no way to tell it is still being written to.
- Pool returns a different, valid server (installations with an active non-direct server): the bytes are written to the receiving server, but the database records a different server. The file then cannot be downloaded, because the application looks for it in the wrong place.
The second outcome is the damaging one, and it is purely a matter of configuration which of the two an installation gets.
Steps to reproduce (Issue 2)
- Set up at least one direct file server and make sure no non-direct server is active.
- Start a large upload to that server, so the browser holds its upload endpoint.
- While the upload is running, set the server to Read Only in the admin area.
- Let the upload finish.
Expected: the upload is rejected, or handed to an active server. Actual: the file is stored on the Read Only server and recorded against it.
The same happens for any client that keeps using a previously obtained upload endpoint, for example a long-running bulk uploader — which is how we first noticed this. One account had accumulated files on six different Read Only servers over a long period, and the behaviour was never explainable from the admin UI.
Suggested fixes
app/helpers/FileServerHelper.class.php
if ($uploadServerOverride) {
- return [
- $uploadServerOverride,
- ];
+ // Only honour the override when that server actually accepts
+ // uploads; otherwise fall through to normal pool selection.
+ $overrideAcceptsUploads = (int) $db->getValue('SELECT COUNT(*) '
+ . 'FROM file_server '
+ . 'WHERE id = :id AND statusId = 2 '
+ . 'LIMIT 1', [
+ 'id' => $uploadServerOverride,
+ ]);
+ if ($overrideAcceptsUploads) {
+ return [
+ $uploadServerOverride,
+ ];
+ }
}
app/services/Uploader.class.php
_storeFile() reports errors via $fileUpload->error and return $fileUpload;, so the failure can be surfaced the same way:
if (!$uploadServerDetails) {
- // if we failed to load any server, fallback on the current server
- $uploadServerDetails = FileHelper::getCurrentServerDetails();
- $uploadServerId = $uploadServerDetails['id'];
+ // Fall back on the receiving server only when it actually accepts
+ // uploads. Otherwise the file would either be recorded against a
+ // Read Only / Disabled server, or - worse - against a different
+ // server than the one holding the bytes, breaking its download.
+ $currentServerDetails = FileHelper::getCurrentServerDetails();
+ if ((int) $currentServerDetails['statusId'] !== 2) {
+ $fileUpload->error = TranslateHelper::t(
+ 'classuploader_no_upload_server_available',
+ 'No file server is currently available to accept uploads. Please try again later.'
+ );
+ return $fileUpload;
+ }
+ $uploadServerDetails = $currentServerDetails;
+ $uploadServerId = $uploadServerDetails['id'];
}
This changes the behaviour from "store it somewhere" to "fail the upload", which we believe is correct, but it is a behavioural change and worth your judgement. A new translation key would be needed, otherwise the English default text is used.
Related observation (not a bug, but worth knowing)
The accountUploadTypes filter is applied only in the pool query, never in the direct-server shortcut. We used that deliberately during maintenance: setting accountUploadTypes to a value matching no account type removes a server from new upload selection, while uploads already in flight still complete correctly on it, because statusId stays 2.
That worked well and was in fact the only safe way we found to drain a server. It does, however, illustrate that the two paths have drifted apart — one honours the filter and the status, the other honours neither.
If the fixes above are applied, a documented "drain" mechanism would be a useful addition: a way to stop new uploads being directed to a server while letting in-flight ones finish and be recorded correctly.
Environment
- Yetishare 6.1.0, upgrade from 5.3.1 tested against a copy of our production database
- 11 direct file servers, 1 local server (disabled)
- Server selection method: Least Used Space
- No geo or account-type restrictions configured on any server
- PHP 8.2, MariaDB 10.3