Skip to content

Fix: null dereference in RestrictionUri::getAssetUrlInformation() when asset has no restriction - #225

Closed
blankse wants to merge 1 commit into
dachcom-digital:masterfrom
blankse:fix/asset-url-info-null-guard
Closed

blankse wants to merge 1 commit into
dachcom-digital:masterfrom
blankse:fix/asset-url-info-null-guard

Conversation

@blankse

@blankse blankse commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Bug

In RestrictionUri::getAssetUrlInformation():

try {
    $restriction = Restriction::getByTargetId($assetId, 'asset');
} catch (\Exception $e) {
    return null;
}

$userGroups = $restriction->getRelatedGroups();

Restriction::getByTargetId() returns null (not a thrown exception) when there is no restriction row for the asset — the common case of an asset in a protected folder without an explicit restriction record. $restriction->getRelatedGroups() then throws \Error, which is not caught by the catch (\Exception) above (Error is not an Exception), so it surfaces as an uncaught fatal.

Fix

Guard the null case (no restriction → no related groups), consistent with the "restricted mode without any restriction settings → empty groups" handling immediately below.

`Restriction::getByTargetId()` returns null (not an exception) when no
restriction row exists for the asset. The following
`$restriction->getRelatedGroups()` then throws an \Error, which is not caught
by the surrounding `catch (\Exception)` -> uncaught fatal. Guard the null case
(no restriction -> no related groups), consistent with the protected-storage
fallback right below.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 20, 2026 •

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@blankse

blankse commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@blankse

blankse commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Withdrawing — on master (v5) this is not a defect. Restriction\Dao::getByField() throws a \Exception when no row exists, so Restriction::getByTargetId(): self never returns null and the not-found case is already handled by the surrounding catch (\Exception) { return null; }. $restriction is therefore always a Restriction here, making the added instanceof redundant (correctly flagged by phpstan). The nullable behaviour only exists in a downstream fork where getByField() returns null instead of throwing. Apologies for the noise.

@blankse blankse closed this Jul 20, 2026
@blankse
blankse deleted the fix/asset-url-info-null-guard branch July 20, 2026 08:11
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 20, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant