Skip to content

Commit 9d442c3

Browse files
committed
Guard deletes, and stop malformed paths reaching the API.
Deleting content was not guarded. unpublishUrl() dispatches QuantEvent::UNPUBLISH, a different event from the one the guard watched, so a delete on an unrecognised host withdrew the matching URL from whichever project the fallback landed on. That is the worst of the three: publishing to the wrong project adds a page, but unpublishing takes a live one down. Worse, the domain was only ever negotiated by Quant's own drush commands. Deleting a node through drush php:eval, a migration, or any other command resolved the base project even with --uri set. Verified: a delete as clienta withdrew /node/8 and /fr/node/8 from the base project rather than the domain's. Negotiation now happens in the guard subscriber, which every publish, redirect and unpublish passes through, so it no longer depends on which entry point started the work. The call is cached per process. Separately, nothing should ever be published at //fr/node/1. The handleInternalPathRedirects fix removed the cause found so far, but paths are assembled from prefixes, base paths and aliases all over the module, and any of them can be empty. Utility::normalizePath() collapses repeated slashes and is applied where routes enter the queue and again at the API boundary for content, redirects and unpublishes. Normalising before the self-redirect check also means a malformed source is recognised as equal to its destination instead of being published as a redirect to itself. quant_cron had one more producer, filtering out the empty default prefix. The query string is left alone: an oEmbed route carries a whole URL in one, and those slashes are not ours to collapse. Regression matrix now covers deletion in both shapes and multilingual: 25 cases, all passing. 76 unit and kernel tests.
1 parent c44a958 commit 9d442c3

6 files changed

Lines changed: 121 additions & 10 deletions

File tree

modules/quant_api/src/EventSubscriber/QuantApi.php

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,11 @@ public static function getSubscribedEvents(): array {
7979
* The redirect event.
8080
*/
8181
public function onRedirect(QuantRedirectEvent $event) {
82-
$source = $event->getSourceUrl();
83-
$dest = $event->getDestinationUrl();
82+
// Normalise before the self-redirect check, so that a malformed source
83+
// like //fr/node/1 is recognised as the same path as its destination
84+
// rather than published as a redirect to itself.
85+
$source = Utility::normalizePath($event->getSourceUrl());
86+
$dest = Utility::normalizePath($event->getDestinationUrl());
8487
$statusCode = $event->getStatusCode();
8588

8689
if ($source == $dest) {
@@ -114,7 +117,10 @@ public function onRedirect(QuantRedirectEvent $event) {
114117
public function onOutput(QuantEvent $event) {
115118

116119
$config = \Drupal::config('quant.settings');
117-
$path = $event->getLocation();
120+
// Last line of defence: a path assembled from an empty prefix or base
121+
// must not reach the API as //fr/node/1, which would publish a duplicate
122+
// resource alongside the real one.
123+
$path = Utility::normalizePath($event->getLocation());
118124
$content = $event->getContents();
119125
$meta = $event->getMetadata();
120126

modules/quant_cron/quant_cron.module

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,9 +284,11 @@ function quant_cron_get_views_routes() {
284284

285285
$paths[] = $path;
286286

287-
// Language negotiation may also provide path prefixes.
287+
// Language negotiation may also provide path prefixes. The default
288+
// language usually has an empty one, which would otherwise produce
289+
// //path.
288290
if ($prefixes = \Drupal::config('language.negotiation')->get('url.prefixes')) {
289-
foreach ($prefixes as $prefix) {
291+
foreach (array_filter($prefixes) as $prefix) {
290292
$paths[] = "/{$prefix}/{$path}";
291293
}
292294
}

src/EventSubscriber/DomainGuardSubscriber.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Drupal\quant\EventSubscriber;
44

5+
use Drupal\quant\CliDomainContext;
56
use Drupal\quant\Event\QuantEvent;
67
use Drupal\quant\Event\QuantRedirectEvent;
78
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
@@ -62,6 +63,11 @@ public static function getSubscribedEvents(): array {
6263
// content would still let a misdirected run rewrite another client's
6364
// redirect map. The publisher listens at -999.
6465
QuantRedirectEvent::UPDATE => ['onOutput', 100],
66+
// Unpublishing is the one that cannot be walked back. Deleting a node
67+
// on an unrecognised host would withdraw the matching URL from
68+
// whichever project the fallback landed on, taking down a live page
69+
// belonging to another client.
70+
QuantEvent::UNPUBLISH => ['onOutput', 100],
6571
];
6672
}
6773

@@ -72,6 +78,14 @@ public static function getSubscribedEvents(): array {
7278
* The content or redirect event.
7379
*/
7480
public function onOutput($event) {
81+
// Every publish, redirect and unpublish passes through here, which makes
82+
// it the one place guaranteed to run no matter what triggered the work.
83+
// Quant's own drush commands negotiate the domain themselves, but a node
84+
// deleted by drush php:eval, a migration, or any other command does not
85+
// reach them, and would resolve the base project instead of the domain's.
86+
// The call is cached per process, so this costs nothing after the first.
87+
CliDomainContext::initialize();
88+
7589
if (!$this->hostIsUnknown($host)) {
7690
return;
7791
}

src/Plugin/QueueItem/RouteItem.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Drupal\quant\Event\QuantEvent;
66
use Drupal\quant\Seed;
7+
use Drupal\quant\Utility;
78

89
/**
910
* A Quant queue item for a redirect.
@@ -49,7 +50,7 @@ public function __construct(array $data = []) {
4950
if (substr($route, 0, 1) != '/') {
5051
$route = "/{$route}";
5152
}
52-
$route = trim($route);
53+
$route = Utility::normalizePath(trim($route));
5354

5455
$this->route = $route;
5556
$this->uri = $data['uri'] ?? strtok($route, '?');

src/Utility.php

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -305,14 +305,43 @@ public static function getPageInfo(?array $urls = NULL) : string {
305305
}
306306

307307
/**
308-
* Unpublish the given URL and optionally log a message.
308+
* Collapses repeated slashes in a path.
309+
*
310+
* Paths are assembled from language prefixes, base paths and aliases, and
311+
* any of those can be empty, which leaves a stray slash behind. A path like
312+
* //fr/node/1 is published as a distinct resource from /fr/node/1, so the
313+
* project accumulates duplicates nobody asked for. Callers should build the
314+
* path correctly; this is the backstop that keeps a malformed one off the
315+
* wire.
316+
*
317+
* @param string $path
318+
* The path, which may carry a query string.
319+
*
320+
* @return string
321+
* The path with repeated slashes collapsed.
322+
*/
323+
public static function normalizePath(string $path) : string {
324+
// Only the path can pick up stray slashes. A query string may legitimately
325+
// contain them, in an oEmbed url for instance, so it is left alone.
326+
$parts = explode('?', $path, 2);
327+
$parts[0] = preg_replace('#/{2,}#', '/', $parts[0]);
328+
329+
if ($parts[0] === '') {
330+
$parts[0] = '/';
331+
}
332+
333+
return implode('?', $parts);
334+
}
335+
336+
/**
337+
* Unpublishes a url from Quant.
309338
*
310339
* @param string $url
311-
* The URL to unpublish.
340+
* The url to unpublish.
312341
* @param string $message
313-
* The message to log.
342+
* Message to log.
314343
* @param bool $log
315-
* Whether or not to log the message.
344+
* Whether to log the action.
316345
*/
317346
public static function unpublishUrl(string $url, string $message = '', bool $log = TRUE) : void {
318347
if (!trim($url)) {
@@ -321,6 +350,8 @@ public static function unpublishUrl(string $url, string $message = '', bool $log
321350
return;
322351
}
323352

353+
$url = self::normalizePath($url);
354+
324355
\Drupal::service('event_dispatcher')->dispatch(new QuantEvent('', $url, [], NULL), QuantEvent::UNPUBLISH);
325356

326357
if ($log) {
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<?php
2+
3+
namespace Drupal\Tests\quant\Unit;
4+
5+
use Drupal\quant\Utility;
6+
use Drupal\Tests\UnitTestCase;
7+
8+
/**
9+
* Ensures malformed paths cannot reach the API.
10+
*
11+
* Paths are assembled from language prefixes, base paths and aliases, any of
12+
* which can be empty. //fr/node/1 is a different resource from /fr/node/1, so
13+
* a stray slash publishes a duplicate nobody asked for.
14+
*
15+
* @coversDefaultClass \Drupal\quant\Utility
16+
*
17+
* @group quant
18+
*/
19+
class UtilityNormalizePathTest extends UnitTestCase {
20+
21+
/**
22+
* Paths and what they should collapse to.
23+
*
24+
* @return array
25+
* Test cases.
26+
*/
27+
public static function pathProvider() : array {
28+
return [
29+
'already correct' => ['/fr/node/1', '/fr/node/1'],
30+
'doubled prefix slash' => ['//fr/node/1', '/fr/node/1'],
31+
'doubled root slash' => ['//node/1', '/node/1'],
32+
'tripled' => ['///node/1', '/node/1'],
33+
'interior double' => ['/fr//node/1', '/fr/node/1'],
34+
'trailing double' => ['/fr/node//', '/fr/node/'],
35+
'root stays root' => ['/', '/'],
36+
'empty becomes root' => ['', '/'],
37+
'query string kept' => ['//fr/search?page=2', '/fr/search?page=2'],
38+
// An oEmbed route carries a whole URL in its query string, and the
39+
// slashes in that URL are not ours to touch.
40+
'slashes in query untouched' => [
41+
'//media/oembed?url=https://example.com/a//b',
42+
'/media/oembed?url=https://example.com/a//b',
43+
],
44+
];
45+
}
46+
47+
/**
48+
* Repeated slashes collapse, without disturbing the query string.
49+
*
50+
* @dataProvider pathProvider
51+
* @covers ::normalizePath
52+
*/
53+
public function testNormalizePath(string $input, string $expected) {
54+
$this->assertEquals($expected, Utility::normalizePath($input));
55+
}
56+
57+
}

0 commit comments

Comments
 (0)