Skip to content

Commit 6770485

Browse files
committed
Guard every write at the client, not only the ones that emit events.
Reviewing the branch for anything the last round missed turned up the same gap a fourth time, in a place events never reach. Search records, facets and index clearing call QuantClient directly and dispatch nothing, so the subscriber never saw them. Clearing is the destructive one: on an unrecognised host it wipes another client's entire search index. Guarding those three call sites would have repeated the mistake, so the decision moved into Drupal\quant\PublishGuard and both the subscriber and the client consult it. The subscriber still runs first, and still stops early enough to skip the render and search work; the client is the backstop that covers a write method nobody has written yet. Applied to the seven methods that change something — send, sendRedirect, sendFile, unpublish, sendSearchRecords, clearSearchIndex, addFacets — and deliberately not to ping, project, search or getUrlMeta, which are reads and are how the settings form reports whether the connection works. Verified against two domains on an unrecognised host: the three search writes refused with zero requests reaching the API, while ping still answers. A refused write returns an empty array, which QuantApi::onOutput could not survive: it read $res['attachments']['js'] straight into array_merge(), which fatals on NULL. The guard makes that reachable, but the API was always entitled to answer without attachments, so the keys are no longer assumed. Static analysis on the changed files also flagged a test constructing QuantPurger with plugin arguments it has no constructor for, and an @var where a @PARAM belonged in the tome batch. 94 unit and kernel tests, 39 regression cases, phpcs clean.
1 parent aaad0ef commit 6770485

7 files changed

Lines changed: 299 additions & 46 deletions

File tree

modules/quant_api/src/Client/QuantClient.php

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Drupal\Core\Config\ConfigFactoryInterface;
66
use Drupal\Core\Logger\LoggerChannelFactoryInterface;
7+
use Drupal\quant\PublishGuard;
78
use Drupal\quant_api\Exception\InvalidPayload;
89
use GuzzleHttp\Client;
910
use GuzzleHttp\Exception\RequestException;
@@ -113,6 +114,30 @@ protected function refreshCredentials() : void {
113114
$this->tlsDisabled = $this->config->get('api_tls_disabled');
114115
}
115116

117+
/**
118+
* Refuses a write the current context cannot be trusted to address.
119+
*
120+
* The event subscriber stops most work earlier, and more cheaply. This
121+
* catches the writes that never dispatch an event — search records, facets
122+
* and index clearing all call this client directly — so a method added
123+
* later is covered without anyone remembering to guard it.
124+
*
125+
* @param string $operation
126+
* What is being attempted, for the log.
127+
*
128+
* @return bool
129+
* TRUE when the caller must not proceed.
130+
*/
131+
protected function refusesWrite(string $operation) : bool {
132+
if (!PublishGuard::refuses($host)) {
133+
return FALSE;
134+
}
135+
136+
PublishGuard::logRefusal($operation, $host);
137+
138+
return TRUE;
139+
}
140+
116141
/**
117142
* Returns the Quant project this client currently targets.
118143
*
@@ -276,6 +301,11 @@ public function search() {
276301
*/
277302
public function send(array $data) : array {
278303
$this->refreshCredentials();
304+
305+
if ($this->refusesWrite('to publish ' . ($data['url'] ?? 'content'))) {
306+
return [];
307+
}
308+
279309
// @todo Switch from 'Quant-Customer' to 'Quant-Organization'.
280310
$response = $this->client->post($this->endpoint, [
281311
RequestOptions::JSON => $data,
@@ -295,6 +325,11 @@ public function send(array $data) : array {
295325
*/
296326
public function sendRedirect(array $data) : array {
297327
$this->refreshCredentials();
328+
329+
if ($this->refusesWrite('to publish a redirect for ' . ($data['url'] ?? 'a url'))) {
330+
return [];
331+
}
332+
298333
// @todo Switch from 'Quant-Customer' to 'Quant-Organization'.
299334
$response = $this->client->post($this->endpoint . '/redirect', [
300335
RequestOptions::JSON => $data,
@@ -315,6 +350,10 @@ public function sendRedirect(array $data) : array {
315350
public function sendFile(string $file, string $url, ?int $rid = NULL) : array {
316351
$this->refreshCredentials();
317352

353+
if ($this->refusesWrite('to upload ' . $url)) {
354+
return [];
355+
}
356+
318357
// Ensure the file is accessible before attempting to send to the API.
319358
if (!file_exists($file) || !is_readable($file) || !is_file($file)) {
320359
throw new InvalidPayload($file);
@@ -363,6 +402,11 @@ public function sendFile(string $file, string $url, ?int $rid = NULL) : array {
363402
*/
364403
public function unpublish(string $url) : array {
365404
$this->refreshCredentials();
405+
406+
if ($this->refusesWrite('to unpublish ' . $url)) {
407+
return [];
408+
}
409+
366410
// @todo Switch from 'Quant-Customer' to 'Quant-Organization'.
367411
$response = $this->client->patch($this->endpoint . '/unpublish', [
368412
'headers' => [
@@ -394,6 +438,7 @@ public function getUrlMeta(array $urls) : array {
394438
'Quant-Url' => $urls,
395439
];
396440
}
441+
397442
// @todo Switch from 'Quant-Customer' to 'Quant-Organization'.
398443
$response = $this->client->post($this->endpoint . '/url-meta', [
399444
RequestOptions::JSON => $urls,
@@ -413,6 +458,11 @@ public function getUrlMeta(array $urls) : array {
413458
*/
414459
public function sendSearchRecords(array $records) : array {
415460
$this->refreshCredentials();
461+
462+
if ($this->refusesWrite('to write ' . count($records) . ' search records')) {
463+
return [];
464+
}
465+
416466
// @todo Switch from 'Quant-Customer' to 'Quant-Organization'.
417467
$response = $this->client->post($this->endpoint . '/search', [
418468
RequestOptions::JSON => $records,
@@ -432,6 +482,11 @@ public function sendSearchRecords(array $records) : array {
432482
*/
433483
public function clearSearchIndex() : array {
434484
$this->refreshCredentials();
485+
486+
if ($this->refusesWrite('to clear the search index')) {
487+
return [];
488+
}
489+
435490
// @todo Switch from 'Quant-Customer' to 'Quant-Organization'.
436491
$response = $this->client->delete($this->endpoint . '/search/all', [
437492
'headers' => [
@@ -450,6 +505,11 @@ public function clearSearchIndex() : array {
450505
*/
451506
public function addFacets(array $facets) : array {
452507
$this->refreshCredentials();
508+
509+
if ($this->refusesWrite('to write search facets')) {
510+
return [];
511+
}
512+
453513
// @todo Switch from 'Quant-Customer' to 'Quant-Organization'.
454514
$response = $this->client->post($this->endpoint . '/search/facet', [
455515
RequestOptions::JSON => $facets,

modules/quant_api/src/EventSubscriber/QuantApi.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,17 @@ public function onOutput(QuantEvent $event) {
157157
return FALSE;
158158
}
159159

160-
$media = array_merge($res['attachments']['js'], $res['attachments']['css'], $res['attachments']['media']['images'], $res['attachments']['media']['documents'], $res['attachments']['media']['video']);
160+
// A refused write returns an empty array, and the API is entitled to
161+
// answer without attachments, so none of these keys can be assumed.
162+
// array_merge() fatals on a NULL argument.
163+
$attachments = $res['attachments'] ?? [];
164+
$media = array_merge(
165+
$attachments['js'] ?? [],
166+
$attachments['css'] ?? [],
167+
$attachments['media']['images'] ?? [],
168+
$attachments['media']['documents'] ?? [],
169+
$attachments['media']['video'] ?? []
170+
);
161171

162172
$queue_factory = QuantQueueFactory::getInstance();
163173
$queue = $queue_factory->get('quant_seed_worker');

modules/quant_purger/tests/src/Kernel/QuantPurgerProjectTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ protected function setUp() : void {
6262
* The resolved project.
6363
*/
6464
protected function resolve(string $domainId) {
65-
$plugin = new QuantPurger([], 'quant', []);
65+
$plugin = new QuantPurger();
6666
$plugin->setContainer($this->container);
6767

6868
$method = new \ReflectionMethod($plugin, 'getProjectForDomain');

modules/quant_tome/src/QuantTomeBatch.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,10 @@ public function pathToUri($file_path) {
207207
/**
208208
* Deploy a file to Quant.
209209
*
210-
* @var \Drupal\quant\Plugin\QueueItem $item
210+
* @param \Drupal\quant\Plugin\QueueItem\QuantQueueItemInterface $item
211211
* The file item to send to Quant API.
212+
* @param array $context
213+
* The batch context.
212214
*/
213215
public function deploy($item, array &$context) {
214216
// Batch operations may run in a forked process that never negotiated a

src/EventSubscriber/DomainGuardSubscriber.php

Lines changed: 5 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Drupal\quant\Event\QuantEvent;
77
use Drupal\quant\Event\QuantFileEvent;
88
use Drupal\quant\Event\QuantRedirectEvent;
9+
use Drupal\quant\PublishGuard;
910
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
1011
use Symfony\Component\HttpFoundation\RequestStack;
1112

@@ -95,15 +96,13 @@ public function onOutput($event) {
9596
// The call is cached per process, so this costs nothing after the first.
9697
CliDomainContext::initialize();
9798

98-
if (!$this->hostIsUnknown($host)) {
99+
if (!PublishGuard::refuses($host, $this->requestStack)) {
99100
return;
100101
}
101102

102-
\Drupal::logger('quant')->error('Refused to publish @path: the host @host matches no domain, so the Domain module fell back to the default and this content would be published to project @project. Add a domain for @host, or correct the Host header reaching Drupal.', [
103-
'@path' => self::describe($event),
104-
'@host' => $host,
105-
'@project' => \Drupal::config('quant_api.settings')->get('api_project') ?: 'unknown',
106-
]);
103+
// Stopping here also skips the render and search work the later
104+
// subscribers would do, which the client-level backstop cannot.
105+
PublishGuard::logRefusal('to publish ' . self::describe($event), $host);
107106

108107
$event->stopPropagation();
109108
}
@@ -129,41 +128,4 @@ protected static function describe($event) : string {
129128
return $event->getSourceUrl();
130129
}
131130

132-
/**
133-
* Determines whether the serving host has no domain record.
134-
*
135-
* @param string|null $host
136-
* Set to the offending host when the check fails.
137-
*
138-
* @return bool
139-
* TRUE when the push must be stopped.
140-
*/
141-
protected function hostIsUnknown(&$host = NULL) : bool {
142-
$host = NULL;
143-
144-
$moduleHandler = \Drupal::moduleHandler();
145-
146-
if (!$moduleHandler->moduleExists('domain')) {
147-
return FALSE;
148-
}
149-
150-
$request = $this->requestStack->getCurrentRequest();
151-
152-
if (!$request) {
153-
return FALSE;
154-
}
155-
156-
$host = $request->getHttpHost();
157-
$storage = \Drupal::entityTypeManager()->getStorage('domain');
158-
159-
// With a single domain there is only one project to publish to, so the
160-
// fallback cannot send content anywhere unexpected. Only a genuine
161-
// multi-domain site can lose a page to another site's project.
162-
if (count($storage->loadMultiple()) < 2) {
163-
return FALSE;
164-
}
165-
166-
return empty($storage->loadByHostname($host));
167-
}
168-
169131
}

src/PublishGuard.php

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
<?php
2+
3+
namespace Drupal\quant;
4+
5+
use Symfony\Component\HttpFoundation\RequestStack;
6+
7+
/**
8+
* Decides whether the current context may write to a Quant project.
9+
*
10+
* The Domain module falls back to the default domain when the request host
11+
* matches no domain record. On a site publishing many clients from one Drupal
12+
* instance, that sends whatever is being written to the default client's
13+
* project instead: a page appears on the wrong site, or worse, one is
14+
* withdrawn from it.
15+
*
16+
* The decision lives here rather than in the event subscriber because not
17+
* everything that writes goes through an event. Search records, facets and
18+
* index clearing call the API client directly, and guarding only the event
19+
* path left those open. Both the subscriber and the client consult this, so a
20+
* new write method is covered without anyone remembering to add a check.
21+
*
22+
* @see \Drupal\quant\EventSubscriber\DomainGuardSubscriber
23+
* @see \Drupal\quant_api\Client\QuantClient
24+
*
25+
* @ingroup quant
26+
*/
27+
class PublishGuard {
28+
29+
/**
30+
* Determines whether writing must be refused in the current context.
31+
*
32+
* @param string|null $host
33+
* Set to the offending host when the check fails.
34+
* @param \Symfony\Component\HttpFoundation\RequestStack $requestStack
35+
* The request stack, where the caller has one injected. Falls back to the
36+
* container otherwise.
37+
*
38+
* @return bool
39+
* TRUE when nothing may be written.
40+
*/
41+
public static function refuses(&$host = NULL, ?RequestStack $requestStack = NULL) : bool {
42+
$host = NULL;
43+
44+
// No container means no domain module, and nothing to get wrong. This
45+
// also keeps unit tests that never build one from fatalling here.
46+
if (!\Drupal::hasContainer()) {
47+
return FALSE;
48+
}
49+
50+
if (!\Drupal::hasService('module_handler') || !\Drupal::moduleHandler()->moduleExists('domain')) {
51+
return FALSE;
52+
}
53+
54+
if (!$requestStack) {
55+
if (!\Drupal::hasService('request_stack')) {
56+
return FALSE;
57+
}
58+
$requestStack = \Drupal::service('request_stack');
59+
}
60+
61+
if (!\Drupal::hasService('entity_type.manager')) {
62+
return FALSE;
63+
}
64+
65+
$request = $requestStack->getCurrentRequest();
66+
67+
if (!$request) {
68+
return FALSE;
69+
}
70+
71+
$host = $request->getHttpHost();
72+
$storage = \Drupal::entityTypeManager()->getStorage('domain');
73+
74+
// With a single domain there is only one project to write to, so the
75+
// fallback cannot reach anywhere unexpected. Only a genuine multi-domain
76+
// site can lose a page to another site's project.
77+
if (count($storage->loadMultiple()) < 2) {
78+
return FALSE;
79+
}
80+
81+
return empty($storage->loadByHostname($host));
82+
}
83+
84+
/**
85+
* Logs a refusal, naming the host and the project that would have received.
86+
*
87+
* @param string $what
88+
* The path, url or operation being refused.
89+
* @param string|null $host
90+
* The host that matched no domain.
91+
*/
92+
public static function logRefusal(string $what, ?string $host) : void {
93+
\Drupal::logger('quant')->error('Refused @what: the host @host matches no domain, so the Domain module fell back to the default and this would have been written to project @project. Add a domain for @host, or correct the Host header reaching Drupal.', [
94+
'@what' => $what,
95+
'@host' => $host ?? 'unknown',
96+
'@project' => \Drupal::config('quant_api.settings')->get('api_project') ?: 'unknown',
97+
]);
98+
}
99+
100+
}

0 commit comments

Comments
 (0)