Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions class/Admin/AccessPolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,22 @@ public static function isAllowed(): bool
* Same three-way decision as isAllowed(), but "admin" is the raw
* $GLOBALS['xoopsUserIsAdmin'] flag rather than xoops_isDeveloperRequest().
* That is deliberate, and it is what the preload has always used to decide
* whether to render the bar. Reusing the strict gate here would diverge on
* two axes that the toolbar does not care about:
* whether to render the bar. Reusing the strict gate here diverged on two
* axes that the toolbar does not care about. One has since closed:
*
* - xoops_isDeveloperRequest() additionally requires XOOPS_GROUP_ADMIN
* membership, so a delegated module admin would see the bar and get 403
* from every button on it;
* - it accepts only debug_mode 1 or 2, while the bar renders for any
* non-zero value — debug_mode 3 (Smarty debug) would render a toolbar
* whose endpoints all refuse.
* from every button on it. STILL OPEN, and not a defect — it is the whole
* reason there are two wrappers. These pages expose site-wide SQL,
* sessions and configuration accumulated across other people's requests;
* the toolbar exposes one request. Collapsing them either locks delegated
* module admins out of the bar or lets them into the history. Do not
* "simplify" this without deciding which of those you want;
* - it accepted only debug_mode 1 or 2, while the bar renders for any
* non-zero value, so debug_mode 3 (Smarty debug) rendered a toolbar whose
* endpoints all refused. CLOSED: XOOPS 2.7.3 widened the gate to any
* non-zero mode, on the argument this module made first — the preference
* names which debug facility, not how much debugging.
*
* The endpoints are still not open: each one additionally requires its own
* server-minted XoopsSecurity token, and they expose only the caller's own
Expand Down
55 changes: 52 additions & 3 deletions class/Analysis/SystemDiagnostics.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public function collect(array $xoopsConfig): array
$this->extensionRow('opcache', extension_loaded('Zend OPcache'), $this->opcacheDetail()),
$this->packageRow('php_debugbar', ['php-debugbar/php-debugbar', 'maximebf/debugbar']),
$this->packageRow('monolog', ['monolog/monolog']),
$this->packageRow('whoops', ['filp/whoops']),
$this->packageRow('whoops', ['filp/whoops'], false, ['xwhoops' => 'filp/whoops']),
$this->packageRow('ray', ['spatie/ray', 'spatie/global-ray'], function_exists('ray')),
$this->packageRow('tracy', ['tracy/tracy']),
$this->errorScreenRow(),
Expand Down Expand Up @@ -113,10 +113,36 @@ private function extensionRow(string $id, bool $available, string $detail = ''):
* Composer metadata is inspected without loading optional runtime classes.
* This prevents an incompatible diagnostics package from breaking this page.
*
* @param list<string> $packages
* $moduleVendors is the fallback, and it exists because Composer\InstalledVersions
* reports the packages of every Composer autoloader REGISTERED in this request -- not
* every package present on disk. The distinction is invisible for DebugBar's own
* dependencies, whose autoloader this page has by definition already registered, and
* decisive for a package in another module's vendor directory loaded lazily.
*
* filp/whoops is the case that exposed it. xwhoops requires its autoloader only once
* it has decided to render, so on every request where it legitimately stood down --
* not a developer request, a different declared owner, the use_xwhoops permission
* withheld -- its packages were absent from that metadata although installed, and this
* row read "Not installed". Reporting a missing dependency for what is actually a
* configuration state sends the administrator to composer for a problem composer
* cannot fix.
*
* So: ask Composer first, and when it has nothing, look on disk before saying no.
*
* The disk test requires the package's own composer.json, not merely the directory. An
* interrupted install, a partial upload or a hand-copied tree can leave the directory
* present and the package unusable, and "Installed" is a worse answer there than "Not
* installed" -- it is the same class of false confidence this fallback exists to
* remove, pointing the other way. No version is reported, because reading one would
* mean parsing that file, and this page's contract is to inspect metadata without
* loading or interpreting optional packages.
*
* @param list<string> $packages
* @param array<string, string> $moduleVendors dirname => package, checked on disk when
* Composer's metadata does not carry it
* @return array{id: string, value: string, status: string, detail: string}
*/
private function packageRow(string $id, array $packages, bool $active = false): array
private function packageRow(string $id, array $packages, bool $active = false, array $moduleVendors = []): array
{
foreach ($packages as $package) {
if (! $this->packageInstalled($package)) {
Expand All @@ -129,6 +155,29 @@ private function packageRow(string $id, array $packages, bool $active = false):
return $this->row($id, $active ? 'Active' : 'Installed', $active ? 'ok' : 'info', $detail);
}

foreach ($moduleVendors as $dirname => $package) {
$path = $this->rootPath . DIRECTORY_SEPARATOR . 'modules'
. DIRECTORY_SEPARATOR . $dirname
. DIRECTORY_SEPARATOR . 'vendor'
. DIRECTORY_SEPARATOR . str_replace('/', DIRECTORY_SEPARATOR, $package);

if (! is_dir($path) || ! is_file($path . DIRECTORY_SEPARATOR . 'composer.json')) {
continue;
}

// Deliberately NOT reported as a problem. "Installed but not registered" is
// the normal resting state of a provider that has not been asked to render,
// and the row answering whether it SHOULD have rendered is errorScreenRow()
// immediately below -- which reads the seam and gives the actual reason.
return $this->row(
$id,
'Installed',
'info',
$package . ' in modules/' . $dirname . '/vendor'
. ' (present on disk; not registered with Composer in this request)'
);
}

return $this->row($id, 'Not installed', 'info');
}

Expand Down
Loading