Addresses ce-code-review findings on the photo-editor media-API work: - P0 (#1): PhotoRenumberer now renumbers EVERY on-disk image, using the client manifest only as preferred ORDER and appending any omitted image at the end. A stale/incomplete `order` (e.g. a second browser tab) previously left an unlisted photo at a target slot for phase-2's rename() to silently overwrite — verified data loss, now impossible. The reorder route inherits the guard; create/reconcile is unchanged. - P2 (#3): unique per-call token in the .reorder-tmp-* name so two concurrent renumbers on one folder can't collide and clobber bytes. - P3 (#7): de-duplicate the manifest so a repeated name can't shift/drop a photo. - P2 (#2): applyReorder + doDelete split the two failure stages — a failed refresh AFTER a committed reorder/delete no longer reverts to a stale or ghost state, it reconciles to disk. A DELETE 404 is treated as success so a retried ghost cell converges. - P2 (#4): both custom routes call requirePermission('api.pages.write') so the GHSA-x7hm API-key scope cap applies (owner already holds it, so the owner-only behaviour is unchanged). - P3 (#8): refresh stale comments (photo-01..NN; drop editLoadPhotos ref). PhotoRenumberer's 7-case unit suite still passes and the data-loss repro now preserves all bytes. Assets rebuilt. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
132 lines
5.9 KiB
PHP
132 lines
5.9 KiB
PHP
<?php
|
|
namespace Grav\Plugin\Shared;
|
|
|
|
/**
|
|
* Single source of truth for the `photo-NN` naming invariant, shared by BOTH
|
|
* paths that establish it so their numbering can never diverge:
|
|
* - cache-on-save's create/edit reconcile (onFormProcessed), and
|
|
* - entry-actions' live reorder route (POST /entry/{slug}/photos/order).
|
|
*
|
|
* Files named in the ordered manifest are renamed to `photo-01..NN` (ZERO-PADDED)
|
|
* in that order. Zero-padding is load-bearing: the published feed lists media in
|
|
* filename order and treats the first as the cover (entry.media.images|first), and
|
|
* lexicographic order only equals numeric order past 9 photos when the index is
|
|
* padded (otherwise photo-1, photo-10, photo-2…). The pad width grows with the
|
|
* set so it stays correct for 100+ photos, while normal entries get `photo-01`.
|
|
*
|
|
* Safety: only files that are ON DISK and carry a known IMAGE extension are ever
|
|
* renamed. A crafted manifest entry naming the entry `.md`, a `.gpx`, or a
|
|
* `.meta.yaml` sidecar is silently skipped — it can never be renamed or clobbered.
|
|
* This guard lives here (not only in the callers) so every caller inherits it.
|
|
*
|
|
* Completeness: renumber() ALWAYS renumbers every image already in $dir, not just
|
|
* the manifest subset. $names only supplies the preferred ORDER; any on-disk image
|
|
* the manifest omits is appended at the end. This makes an incomplete/stale
|
|
* manifest (e.g. a second browser tab whose list predates a change) harmless —
|
|
* without it, an unlisted image left sitting at a target slot would be silently
|
|
* OVERWRITTEN (destroyed) by the second-phase rename. The reorder route trusts a
|
|
* client-supplied list, so this guard is what keeps it from losing photos.
|
|
*/
|
|
class PhotoRenumberer
|
|
{
|
|
/** Image extensions eligible for renumbering. Broad on purpose: existing
|
|
* entries may hold .heic even though new uploads are jpg/jpeg/png/webp. */
|
|
private const IMAGE_EXTS = ['jpg', 'jpeg', 'png', 'gif', 'webp', 'heic', 'heif'];
|
|
|
|
/**
|
|
* Renumber every image in $dir to photo-01..NN, using $names as the preferred
|
|
* order and appending any unlisted on-disk images at the end.
|
|
*
|
|
* Two-phase via temp names so a target (photo-02.jpg) can't clobber a
|
|
* not-yet-moved source of the same name (e.g. a straight swap or the
|
|
* un-padded photo-N → photo-0N normalisation pass). Non-image and missing
|
|
* files in $names are skipped and do not consume an index; a repeated name is
|
|
* counted once. Because every on-disk image becomes a target (see the
|
|
* completeness note on the class), the surviving images are numbered
|
|
* contiguously from 01 and no untouched file is ever overwritten.
|
|
*
|
|
* Idempotent: a file already at its correct padded name is left untouched,
|
|
* so re-running with the same manifest (e.g. an auto-retried reorder) is a
|
|
* no-op.
|
|
*/
|
|
public static function renumber(string $dir, array $names): void
|
|
{
|
|
// Keep only real image files, in the requested order, de-duplicated —
|
|
// this is both the security filter and what determines the pad width.
|
|
$targets = [];
|
|
$seen = [];
|
|
foreach ($names as $name) {
|
|
if (!is_string($name) || $name === '') {
|
|
continue;
|
|
}
|
|
$base = basename(str_replace('\\', '/', $name));
|
|
if (isset($seen[$base])) {
|
|
continue; // a repeated name must not consume a second index
|
|
}
|
|
$src = $dir . DIRECTORY_SEPARATOR . $base;
|
|
if (!is_file($src)) {
|
|
continue; // not on disk — skip (idempotent for auto-retry)
|
|
}
|
|
$ext = strtolower(pathinfo($base, PATHINFO_EXTENSION));
|
|
if (!in_array($ext, self::IMAGE_EXTS, true)) {
|
|
continue; // never rename the entry .md, a .gpx, or a sidecar
|
|
}
|
|
$seen[$base] = true;
|
|
$targets[] = [$src, $ext ?: 'jpg'];
|
|
}
|
|
|
|
// Completeness guard: append EVERY other image already in $dir that the
|
|
// manifest didn't list (natural name order), so an incomplete/stale
|
|
// manifest can't leave an unlisted image at a target slot for phase-2 to
|
|
// overwrite. Hidden files ('.'-prefixed temp/sidecar) are never targets.
|
|
$extra = [];
|
|
foreach (@scandir($dir) ?: [] as $f) {
|
|
if ($f === '' || $f[0] === '.' || isset($seen[$f])) {
|
|
continue;
|
|
}
|
|
$p = $dir . DIRECTORY_SEPARATOR . $f;
|
|
if (!is_file($p)) {
|
|
continue;
|
|
}
|
|
$ext = strtolower(pathinfo($f, PATHINFO_EXTENSION));
|
|
if (!in_array($ext, self::IMAGE_EXTS, true)) {
|
|
continue;
|
|
}
|
|
$extra[$f] = [$p, $ext ?: 'jpg'];
|
|
}
|
|
if ($extra) {
|
|
uksort($extra, 'strnatcasecmp');
|
|
foreach ($extra as $t) {
|
|
$targets[] = $t;
|
|
}
|
|
}
|
|
|
|
$width = max(2, strlen((string) count($targets)));
|
|
|
|
// Unique per-call token in the temp name so two concurrent renumbers on
|
|
// the same folder can't collide on a shared '.reorder-tmp-N' path and
|
|
// overwrite one photo's bytes.
|
|
$token = bin2hex(random_bytes(4));
|
|
|
|
$planned = [];
|
|
$i = 1;
|
|
foreach ($targets as [$src, $ext]) {
|
|
$index = str_pad((string) $i, $width, '0', STR_PAD_LEFT);
|
|
$final = $dir . DIRECTORY_SEPARATOR . 'photo-' . $index . '.' . $ext;
|
|
if ($src === $final) {
|
|
$i++;
|
|
continue; // already correctly named — leave it
|
|
}
|
|
$tmp = $dir . DIRECTORY_SEPARATOR . '.reorder-tmp-' . $token . '-' . $i . '.' . $ext;
|
|
@rename($src, $tmp);
|
|
$planned[] = [$tmp, $final];
|
|
$i++;
|
|
}
|
|
foreach ($planned as [$tmp, $final]) {
|
|
if (is_file($tmp)) {
|
|
@rename($tmp, $final);
|
|
}
|
|
}
|
|
}
|
|
}
|