mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-17 00:55:07 +00:00
a5496d24cd
`FileDelivery::describe()` from a console returned
`{"method":"php","detected":true}` on every installation, whatever its web
server. detect() reads SERVER_SOFTWARE, which only exists inside a
request, so a console process has nothing to look at and falls to the
`php` default — and `detected: true` then vouched for it.
The value is right for that process and wrong as a statement about the
installation, which is how anybody running it from `artisan tinker` will
read it. Somebody verifying a healthy nginx tenant hit exactly that, spent
an afternoon on it, and only recognised it as an artefact of *where* the
question was asked after reading `nginx -T` in the container.
There is now a third field. `observed` is false only outside a request,
where `method` is a default rather than a finding. Both screens that read
this run in a request and always see true; it exists for whoever asks from
a shell, which is the one place the answer could mislead.
The two web paths are unchanged and were never wrong — `projectsend:status`
does not report delivery at all, so no fleet ever reported this
incorrectly. What was wrong was a confident answer to a question that
could not be answered from where it was asked.
Three tests: a console reading says not observed and still says php,
because php is what that process would actually do; a reading during a
request observes nginx; and a stated method is observed wherever it is
read, since a decision needs nothing detected to be true.
Found by the session verifying the 2.4.0 canary, not by me.
274 lines
11 KiB
PHP
274 lines
11 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Modules\Files\Delivery;
|
|
|
|
use Illuminate\Http\Request;
|
|
use Illuminate\Http\Response;
|
|
use Illuminate\Support\Facades\Storage;
|
|
use Symfony\Component\HttpFoundation\BinaryFileResponse;
|
|
|
|
/**
|
|
* Puts a file that lives on this server's local disk on the wire.
|
|
*
|
|
* The single place that knows how the bytes travel. Four routes used to
|
|
* decide that for themselves and all four hard-coded nginx's header, so
|
|
* an Apache or LiteSpeed installation served four different flavours of
|
|
* empty response — uploads worked, thumbnails were broken images, and
|
|
* downloads arrived as 0 bytes. Callers now say *what* to send and this
|
|
* decides *how*.
|
|
*
|
|
* It authorizes nothing. Every caller has already done that its own way
|
|
* — a policy, a share token, a public-listing check — and the path it
|
|
* passes is always derived from a row it just authorized, never from the
|
|
* request. That is load-bearing: `serve()` will send any file under the
|
|
* storage root, so a caller that passed user input would have built a
|
|
* file-disclosure bug. The root check below is the backstop, not the
|
|
* rule.
|
|
*
|
|
* ### Choosing the method
|
|
*
|
|
* `PROJECTSEND_FILE_DELIVERY` picks one explicitly. Left at `auto` — the
|
|
* default — nginx gets its own fast path and everything else gets PHP
|
|
* streaming.
|
|
*
|
|
* Auto deliberately never chooses `xsendfile`. Apache's `mod_xsendfile`
|
|
* needs `XSendFilePath` to whitelist the storage directory as well as
|
|
* being loaded, and nothing here can see whether it does; choosing it
|
|
* because the module is present would swap a silent failure anybody can
|
|
* diagnose from the dashboard for one nobody can. So it stays something
|
|
* an operator turns on having configured it.
|
|
*
|
|
* A value that is not a method falls back to auto rather than throwing.
|
|
* A typo in an environment variable should cost speed, not every
|
|
* download on the installation.
|
|
*/
|
|
class FileDelivery
|
|
{
|
|
/**
|
|
* The disk uploads live on. Named rather than injected because the
|
|
* whole class is about the local-disk case: a file on S3 never
|
|
* reaches here, it is a signed redirect from StoredFileResponse.
|
|
*/
|
|
private const DISK = 'files';
|
|
|
|
/** The internal nginx location that maps back onto the storage root. */
|
|
private const NGINX_LOCATION = '/protected-files/';
|
|
|
|
public function __construct(private readonly Request $request) {}
|
|
|
|
/**
|
|
* The method in force, and whether it was detected or stated.
|
|
*
|
|
* @return array{method: DeliveryMethod, detected: bool, observed: bool}
|
|
*/
|
|
public function resolve(): array
|
|
{
|
|
$configured = config('projectsend.file_delivery');
|
|
$explicit = is_string($configured) ? DeliveryMethod::tryFrom($configured) : null;
|
|
|
|
if ($explicit !== null) {
|
|
return ['method' => $explicit, 'detected' => false, 'observed' => true];
|
|
}
|
|
|
|
// Whether there was anything to detect *from*. detect() reads
|
|
// SERVER_SOFTWARE, which only exists inside a request — so a
|
|
// console process has nothing to look at and falls to the `php`
|
|
// default. That default is right for the console (no web server is
|
|
// handling this, so nothing could hand a file off), and wrong as a
|
|
// statement about the installation, which is how somebody reading
|
|
// it from `artisan tinker` will take it.
|
|
//
|
|
// Reported rather than papered over: a reader who runs
|
|
// `describe()` from a shell on a perfectly good nginx box was
|
|
// being told `php`, with `detected: true` vouching for it. That
|
|
// cost somebody an afternoon before it was recognised as an
|
|
// artefact of asking outside a request.
|
|
$observed = is_string($this->request->server('SERVER_SOFTWARE'));
|
|
|
|
return ['method' => $this->detect(), 'detected' => true, 'observed' => $observed];
|
|
}
|
|
|
|
public function method(): DeliveryMethod
|
|
{
|
|
return $this->resolve()['method'];
|
|
}
|
|
|
|
/**
|
|
* The same answer as a plain array, for a screen or a probe.
|
|
*
|
|
* Spelled out rather than leaning on a backed enum encoding itself,
|
|
* because this shape is read by the dashboard and by whatever watches
|
|
* the installation from outside, and neither should change meaning if
|
|
* the enum ever grows a JsonSerializable of its own.
|
|
*
|
|
* `observed` is false only outside an HTTP request, where nothing can
|
|
* be detected and `method` is a default rather than a finding. Both
|
|
* screens that read this run in a request, so they always see true;
|
|
* it exists for whoever asks from a console.
|
|
*
|
|
* @return array{method: string, detected: bool, observed: bool}
|
|
*/
|
|
public function describe(): array
|
|
{
|
|
$resolved = $this->resolve();
|
|
|
|
return [
|
|
'method' => $resolved['method']->value,
|
|
// True when nobody said which to use. The distinction matters
|
|
// to the reader: a detected `php` is an installation that
|
|
// could be faster, a stated one is somebody's decision.
|
|
'detected' => $resolved['detected'],
|
|
// And whether the detection had anything to work with.
|
|
'observed' => $resolved['observed'],
|
|
];
|
|
}
|
|
|
|
/**
|
|
* What the server says it is.
|
|
*
|
|
* `SERVER_SOFTWARE` is set by the web server itself through the
|
|
* FastCGI parameters, so it describes the process actually holding
|
|
* the connection to PHP. That is the right thing to ask: the header
|
|
* has to be understood by *that* server, not by whatever sits in
|
|
* front of it.
|
|
*
|
|
* The known-wrong case is nginx reverse-proxying Apache, which
|
|
* INSTALL.md offers as a way to keep an existing Apache. This reads
|
|
* Apache and picks PHP streaming, so downloads work and are slower
|
|
* than they need to be — the safe direction, and the reason the
|
|
* override exists.
|
|
*/
|
|
private function detect(): DeliveryMethod
|
|
{
|
|
$software = $this->request->server('SERVER_SOFTWARE');
|
|
$software = strtolower(is_string($software) ? $software : '');
|
|
|
|
return str_contains($software, 'nginx') ? DeliveryMethod::Nginx : DeliveryMethod::Php;
|
|
}
|
|
|
|
/**
|
|
* @param string $path disk-relative, and always derived from an
|
|
* already-authorized row — never from the request
|
|
* @param int|null $length when the caller already knows it; PHP
|
|
* streaming ignores it and measures the file
|
|
*/
|
|
public function serve(string $path, string $mimeType, string $disposition, ?int $length = null): Response|BinaryFileResponse
|
|
{
|
|
$this->assertRelative($path);
|
|
|
|
$headers = array_filter([
|
|
'Content-Type' => $mimeType,
|
|
'Content-Disposition' => $disposition,
|
|
'Content-Length' => $length === null ? null : (string) $length,
|
|
], static fn (?string $value): bool => $value !== null);
|
|
|
|
return match ($this->method()) {
|
|
DeliveryMethod::Nginx => response('', 200, [
|
|
'X-Accel-Redirect' => self::NGINX_LOCATION.$path,
|
|
...$headers,
|
|
]),
|
|
DeliveryMethod::XSendFile => response('', 200, [
|
|
// An absolute filesystem path, unlike nginx's URL path.
|
|
// Renaming the header without changing the value is the
|
|
// obvious way to "add Apache support" and produces a
|
|
// second broken install.
|
|
'X-Sendfile' => $this->absolutePathWithin($path),
|
|
...$headers,
|
|
]),
|
|
DeliveryMethod::Php => $this->stream($this->absolutePathWithin($path), $headers),
|
|
};
|
|
}
|
|
|
|
/**
|
|
* @param array<string, string> $headers
|
|
*/
|
|
private function stream(string $absolute, array $headers): BinaryFileResponse
|
|
{
|
|
// A large download can outlive max_execution_time, and the visitor
|
|
// sees a truncated file rather than an error. The web server is
|
|
// not holding this one open for us.
|
|
if (function_exists('set_time_limit')) {
|
|
@set_time_limit(0);
|
|
}
|
|
|
|
// BinaryFileResponse rather than a hand-written readfile loop: it
|
|
// answers Range requests, which is what makes seeking through a
|
|
// long video work. nginx does that for itself on the fast path, so
|
|
// rolling our own here would break preview scrubbing on exactly
|
|
// the installations this fallback exists for.
|
|
//
|
|
// Content-Length is deliberately dropped from the headers: the
|
|
// response sets its own from the file, and a caller's figure that
|
|
// disagrees — a stale `files.size`, or a range being served —
|
|
// truncates the download.
|
|
unset($headers['Content-Length']);
|
|
|
|
return new BinaryFileResponse($absolute, 200, $headers);
|
|
}
|
|
|
|
/**
|
|
* The path must stay a path *inside* the storage area.
|
|
*
|
|
* Checked for every method, and without touching the filesystem,
|
|
* because nginx resolves `..` in the URL it is handed just as
|
|
* happily as a filesystem call would -- and because every method
|
|
* puts this value into a response header. Callers pass paths from rows
|
|
* they authorized rather than from the request, so this is a
|
|
* backstop; it is here because the cost of being wrong about that,
|
|
* once, is handing over any file the web server can read.
|
|
*/
|
|
private function assertRelative(string $path): void
|
|
{
|
|
abort_if(
|
|
$path === ''
|
|
|| str_starts_with($path, '/')
|
|
|| preg_match('#(^|/)\.\.(/|$)#', $path) === 1
|
|
// A control character in the path is header injection, not
|
|
// traversal: this value is written into X-Accel-Redirect or
|
|
// X-Sendfile, and a CR or LF in a header value splits the
|
|
// response. PHP's header() refuses to emit one, so the real
|
|
// effect is a 500 on every download, preview and thumbnail
|
|
// of that file rather than a split -- a file permanently
|
|
// broken by its own name.
|
|
//
|
|
// Paths are `Y/m/{uuid}.{ext}` and generated here, so this
|
|
// should be unreachable. It is checked because the
|
|
// extension is not: it is taken from the uploader's
|
|
// filename, and on a migrated installation from a v1
|
|
// database, which is somebody else's data.
|
|
|| preg_match('/[\x00-\x1F\x7F]/', $path) === 1,
|
|
404,
|
|
);
|
|
}
|
|
|
|
/**
|
|
* The absolute path, proven to resolve inside the storage root.
|
|
*
|
|
* Only the two methods that hand over a *filesystem* path need this,
|
|
* and only they can afford it: it resolves symlinks, so it answers
|
|
* the question `assertRelative()` cannot — whether the file is really
|
|
* where the path says it is.
|
|
*
|
|
* It also requires the file to exist, which is why nginx does not go
|
|
* through it. On that path PHP never opens the file, and adding a
|
|
* stat to every download to discover something nginx is about to
|
|
* discover anyway would be a cost with no answer attached.
|
|
*/
|
|
private function absolutePathWithin(string $path): string
|
|
{
|
|
$disk = Storage::disk(self::DISK);
|
|
|
|
$absolute = realpath($disk->path($path));
|
|
$root = realpath($disk->path(''));
|
|
|
|
abort_if(
|
|
$absolute === false || $root === false || ! str_starts_with($absolute, rtrim($root, '/').'/'),
|
|
404,
|
|
);
|
|
|
|
return $absolute;
|
|
}
|
|
}
|