Skip to content

fix(photos): preserve source page after photo deletion #1777

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mrksbgg wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(photos): preserve source page after photo deletion #1777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7937f7c
fix(photos): preserve source page after photo deletion
mrksbgg Sep 8, 2026
c7ea869
Merge branch 'OpenVK:master' into fix/photo-delete-redirect
mrksbgg Sep 8, 2026
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
30 changes: 27 additions & 3 deletions Web/Presenters/PhotosPresenter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,10 +410,34 @@ public function renderDeletePhoto(int $ownerId, int $photoId): void
$this->flashFail("err", tr("error_access_denied_short"), tr("error_access_denied"));
}

if (!is_null($album = $photo->getAlbum())) {
$redirect = null;
$returnTo = $this->queryParam("return_to");

if (is_string($returnTo) && $returnTo !== "") {
$parsedReturnTo = parse_url($returnTo);
if ($parsedReturnTo !== false && !isset($parsedReturnTo["host"]) && str_starts_with($returnTo, "/") && !str_starts_with($returnTo, "//")) {

@fayzetwin1 fayzetwin1 Sep 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

я бы предпочел брать путь более строгим путем или проверять регуляркой типа:
!str_starts_with($returnTo, '//') && !str_starts_with($returnTo, '/\\')

насколько я помню parse_url в PHP в некоторых случаях работает не всегда точно
то есть, грубо говоря, я бы добавил еще проверку на !str_starts_with($returnTo, "/\\")

$redirect = $returnTo;
} elseif ($parsedReturnTo !== false && isset($parsedReturnTo["host"])) {

@fayzetwin1 fayzetwin1 Sep 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

по сути оно делает то же самое - берет сугубо $parsedReturnTo["path"] и отбрасывает хост

для простоты я считаю можно просто разрешать только безопасные относительные пути без протоколов и доменов

к примеру:

   if (
       is_string($returnTo) 
       && str_starts_with($returnTo, "/") 
       && !str_starts_with($returnTo, "//") 
       && !str_starts_with($returnTo, "/\\")
   ) {
       $parsed = parse_url($returnTo);
       if ($parsed !== false && !isset($parsed["scheme"]) && !isset($parsed["host"])) {
           $redirect =$returnTo;
       }
   }

$currentHost = strtolower((string) ($_SERVER["HTTP_HOST"] ?? ""));
$returnHost = strtolower($parsedReturnTo["host"] . (isset($parsedReturnTo["port"]) ? ":" . $parsedReturnTo["port"] : ""));
if ($currentHost !== "" && $returnHost === $currentHost) {
$redirect = $parsedReturnTo["path"] ?? "/";
if (isset($parsedReturnTo["query"])) {
$redirect .= "?" . $parsedReturnTo["query"];
}
if (isset($parsedReturnTo["fragment"])) {
$redirect .= "#" . $parsedReturnTo["fragment"];
}
}
}
}

if ($redirect === null && !is_null($album = $photo->getAlbum())) {
$redirect = '/album' . $album->getPrettyId();
} else {
$redirect = "/id0";
}

if ($redirect === null) {
$redirect = $photo->getOwner()->getURL();
}

$photo->isolate();
Expand Down
2 changes: 1 addition & 1 deletion Web/Presenters/templates/Photos/Photo.latte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
</a>
<div n:if="isset($thisUser) && $thisUser->getId() === $photo->getOwner()->getId()">
<a href="/photo{$photo->getPrettyId()}/edit" class="profile_link" style="display:block;width:96%;">{_edit}</a>
<a id="_photoDelete" href="/photo{$photo->getPrettyId()}/delete" class="profile_link" style="display:block;width:96%;">{_delete}</a>
<a id="_photoDelete" href="/photo{$photo->getPrettyId()}/delete{ifset $_SERVER['HTTP_REFERER']}?return_to={rawurlencode($_SERVER['HTTP_REFERER'])}{/ifset}" class="profile_link" style="display:block;width:96%;">{_delete}</a>

@fayzetwin1 fayzetwin1 Sep 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

реферер может содержать чисто в теории что-то лишнее (внешний сайт условно говоря), и ссылка в шаблоне может тогда содержать лишний мусор

мне кажется реферер можно определять прямо в PhotosPresenter через $this->getHttpRequest()->getReferer() как fallback если мы не передали return_to

</div>
<a href="{$photo->getURL()}" class="profile_link" target="_blank" style="display:block;width:96%;">{_"open_original"}</a>
<a n:if="isset($thisUser) && $thisUser->getId() != $photo->getOwner()->getId()" class="profile_link" style="display:block;width:96%;" href="javascript:reportPhoto({$photo->getId()})">{_report}</a>
Expand Down