FeedMalformedPayloadTest.php — Reviews Feed – Add Testimonials and Customer Reviews From Google Reviews, Yelp, TripAdvisor, and More 2.6.7

reviews-feed / tests / Unit / FeedMalformedPayloadTest.php

reviews-feed / tests / Unit Last commit date

FeedMalformedPayloadTest.php

361 lines

1	<?php
2
3	namespace SmashBalloon\Reviews\Tests\Unit;
4
5	use PHPUnit\Framework\TestCase;
6	use SmashBalloon\Reviews\Pro\Feed as ProFeed;
7	use SmashBalloon\Reviews\Common\Feed as CommonFeed;
8	use SmashBalloon\Reviews\Common\Util;
9
10	// SinglePostCache / MediaFinder reference this plugin constant at load time;
11	// the plugin defines it in bootstrap.php, which the unit-test bootstrap does
12	// not load. Define it so the classes autoload and the pre-fix run reaches the
13	// real `$single_review['source']` string-offset TypeError (not a load error).
14	if (!defined('SBR_POSTS_TABLE')) {
15	define('SBR_POSTS_TABLE', 'sbr_reviews_posts');
16	}
17
18	/**
19	* SMASH-1578 regression coverage.
20	*
21	* A customer on PHP 8.4 (poseidonpoolsandlandscape.ca, Reviews Feed Pro 2.6.2)
22	* hit a front-end fatal:
23	*
24	* Uncaught TypeError: Cannot access offset of type string on string
25	* in class/Pro/Feed.php:90
26	* #0 class/Common/Feed.php(252): ...->cache_single_posts_from_set(Array, 'ChIJ...')
27	*
28	* Root cause: cache_single_posts_from_set() / find_and_resize_media() assume
29	* every element of the reviews payload is an array and immediately do
30	* `new MediaFinder($single_review['source'])`. When the relay returns a
31	* malformed / error-shaped reviews payload (the customer's debug log shows
32	* reviewsSourceNotCreated 404 / invalidToken 401 / reviewsLicenseNotValid 403
33	* for that place_id), an entry can be a scalar string. Accessing a string
34	* offset by a string key is a hard TypeError on PHP 8.0+ — the customer's 8.4
35	* stack just surfaced a latent crash, it is not 8.4-specific.
36	*
37	* These tests feed a malformed (scalar-only) payload through the real methods.
38	* Pre-fix they throw TypeError on the first iteration. Post-fix every non-array
39	* entry is skipped, so no SinglePostCache / MediaFinder is ever constructed and
40	* the methods return cleanly without a fatal.
41	*/
42	class FeedMalformedPayloadTest extends TestCase
43	{
44	/** @return list<mixed> A payload where every entry is a non-array scalar. */
45	private function malformed_payload(): array
46	{
47	return ['ChIJJZ44LtE9O4gRzgkq8Gh6KWk', 12345, null, false, ''];
48	}
49
50	/**
51	* Array-shaped entries whose 'source' is missing or a scalar string. The
52	* first would emit an "Undefined array key" warning; the second is a real
53	* fatal in find_and_resize_media (direct read $single_review['source']['id']
54	* → string offset). Both must be skipped. (SMASH-1578 / PR #478 review.)
55	*
56	* @return list<array<string,mixed>>
57	*/
58	private function bad_source_payload(): array
59	{
60	return [
61	['text' => 'no source key at all', 'rating' => 5],
62	['text' => 'scalar source', 'source' => 'ChIJJZ44LtE9O4gRzgkq8Gh6KWk'],
63	];
64	}
65
66	/**
67	* Pro::cache_single_posts_from_set — the exact reported crash site
68	* (Pro/Feed.php:90, `new MediaFinder($single_review['source'])`).
69	*/
70	public function test_pro_cache_single_posts_from_set_skips_non_array_entries(): void
71	{
72	$feed = (new \ReflectionClass(ProFeed::class))->newInstanceWithoutConstructor();
73
74	$feed->cache_single_posts_from_set($this->malformed_payload(), 'ChIJJZ44LtE9O4gRzgkq8Gh6KWk');
75
76	$this->assertTrue(true, 'malformed payload did not fatal in Pro::cache_single_posts_from_set');
77	}
78
79	/**
80	* Pro::find_and_resize_media — same `$single_review['source']` pattern
81	* (Pro/Feed.php:56-57), reachable from the media-finding cron path.
82	*/
83	public function test_pro_find_and_resize_media_skips_non_array_entries(): void
84	{
85	$feed = (new \ReflectionClass(ProFeed::class))->newInstanceWithoutConstructor();
86
87	$result = $feed->find_and_resize_media($this->malformed_payload());
88
89	$this->assertIsArray($result, 'malformed payload did not fatal in Pro::find_and_resize_media');
90	}
91
92	/**
93	* find_and_resize_media does a direct nested read $single_review['source']['id'],
94	* so it must skip array entries whose 'source' is missing or a scalar string
95	* (a string offset would fatal). cache_single_posts_from_set intentionally does
96	* NOT skip these — MediaFinder handles a scalar/missing source safely there, and
97	* skipping would drop otherwise-cacheable reviews (PR #478 review follow-up).
98	*/
99	public function test_pro_find_and_resize_media_skips_bad_source_entries(): void
100	{
101	$feed = (new \ReflectionClass(ProFeed::class))->newInstanceWithoutConstructor();
102
103	$result = $feed->find_and_resize_media($this->bad_source_payload());
104
105	$this->assertIsArray($result, 'missing/scalar source did not fatal in find_and_resize_media');
106	}
107
108	/**
109	* Common::add_source_to_post_set runs upstream (in api_request) and writes a
110	* 'source' offset onto each review entry. A string entry in a 0-indexed list
111	* would make that write a fatal string-offset assignment (SMASH-1578 / PR #478
112	* Sentry review). Non-array entries must be skipped; array entries still get
113	* their source stamped.
114	*/
115	public function test_add_source_to_post_set_skips_non_array_entries(): void
116	{
117	$feed = (new \ReflectionClass(CommonFeed::class))->newInstanceWithoutConstructor();
118
119	$source = ['info' => ['id' => 'ChIJJZ44LtE9O4gRzgkq8Gh6KWk', 'url' => 'https://example.test'], 'account_id' => 'ChIJJZ44LtE9O4gRzgkq8Gh6KWk'];
120	$post_set = ['data' => ['reviews' => [
121	['text' => 'valid one'],
122	'an error-shaped string entry',
123	['text' => 'valid two'],
124	]]];
125
126	$result = $feed->add_source_to_post_set($source, $post_set);
127	$reviews = $result['data']['reviews'];
128
129	$this->assertIsArray($reviews[0]['source'], 'array entry should be stamped with source');
130	$this->assertSame('an error-shaped string entry', $reviews[1], 'string entry left untouched, no fatal');
131	$this->assertIsArray($reviews[2]['source'], 'later array entry still stamped');
132	}
133
134	/**
135	* The reviews CONTAINER itself can be a scalar on an error-shaped payload
136	* (e.g. 'reviews' => 'error message'). isset($reviews[0]) is fooled by
137	* string-offset semantics, so the loop must be guarded by an is_array check
138	* on the container or `foreach` emits a warning (this suite fails on
139	* warnings). Returns the post_set untouched. (PR #478 review follow-up.)
140	*/
141	public function test_add_source_to_post_set_handles_scalar_reviews_container(): void
142	{
143	$feed = (new \ReflectionClass(CommonFeed::class))->newInstanceWithoutConstructor();
144
145	$source = ['info' => ['id' => 'ChIJ', 'url' => ''], 'account_id' => 'ChIJ'];
146	$post_set = ['data' => ['reviews' => 'error message from the relay']];
147
148	$result = $feed->add_source_to_post_set($source, $post_set);
149
150	$this->assertSame('error message from the relay', $result['data']['reviews'], 'scalar reviews container returned untouched, no warning');
151	}
152
153	/**
154	* Common::cache_single_posts_from_set — the Free-side variant of the loop
155	* must be equally defensive.
156	*/
157	public function test_common_cache_single_posts_from_set_skips_non_array_entries(): void
158	{
159	$feed = (new \ReflectionClass(CommonFeed::class))->newInstanceWithoutConstructor();
160
161	$feed->cache_single_posts_from_set($this->malformed_payload(), 'ChIJJZ44LtE9O4gRzgkq8Gh6KWk');
162
163	$this->assertTrue(true, 'malformed payload did not fatal in Common::cache_single_posts_from_set');
164	}
165
166	/**
167	* SMASH-1587: a review whose 'provider' is a scalar slug (e.g. 'google')
168	* instead of ['name' => 'google'] crashed the front end — every
169	* SinglePostCache read of $post['provider']['name'] (resize_avatar:146,
170	* resize_image:88, store:293, …) is a "Cannot access offset of type string
171	* on string" fatal on PHP 8. Both SinglePostCache constructors now route
172	* post_data through normalize_review_shape(), coercing a string provider
173	* into the array shape the cache + display code expects. SMASH-1578 guarded
174	* the review + source shapes but not provider, so 2.6.3 still crashed.
175	*
176	* Single source of truth: Util::normalize_review_shape() — used by both
177	* SinglePostCache constructors AND every raw/DB-decoded read site
178	* (PostAggregator dedup, parse_single_review, duplicate_collection).
179	*/
180	private function normalizeProvider($input)
181	{
182	return Util::normalize_review_shape($input);
183	}
184
185	public function test_string_provider_is_coerced_to_array_shape(): void
186	{
187	$out = $this->normalizeProvider([
188	'provider' => 'google',
189	'review_id' => 'abc',
190	'reviewer' => ['name' => 'Jane'],
191	]);
192
193	$this->assertSame(['name' => 'google'], $out['provider'], 'scalar provider slug wrapped as [name => slug]');
194	// Now the previously-fatal read is a safe array access.
195	$this->assertSame('google', $out['provider']['name']);
196	}
197
198	public function test_array_provider_is_left_untouched(): void
199	{
200	$provider = ['name' => 'yelp', 'id' => 'biz-123'];
201	$out = $this->normalizeProvider(['provider' => $provider, 'review_id' => 'x']);
202
203	$this->assertSame($provider, $out['provider'], 'already-correct array provider is not altered');
204	}
205
206	public function test_missing_or_nonstring_provider_becomes_empty_named_array(): void
207	{
208	$missing = $this->normalizeProvider(['review_id' => 'x']);
209	$this->assertSame(['name' => ''], $missing['provider'], 'absent provider gets a safe [name => ""] shape');
210
211	$numeric = $this->normalizeProvider(['provider' => 123, 'review_id' => 'x']);
212	$this->assertSame(['name' => ''], $numeric['provider'], 'non-string scalar provider falls back to [name => ""]');
213	}
214
215	/**
216	* PR #484 Copilot review: an array provider that is missing 'name' (or has a
217	* non-string name) must still come out with a present string name — otherwise
218	* the $review['provider']['name'] reads hit "Undefined array key" notices and
219	* produce empty/incorrect dedup keys. Other provider keys are preserved.
220	*/
221	public function test_array_provider_missing_name_gets_empty_string_name(): void
222	{
223	$missingName = $this->normalizeProvider(['provider' => ['id' => 'biz-1'], 'review_id' => 'x']);
224	$this->assertSame('', $missingName['provider']['name'], 'missing name filled with empty string');
225	$this->assertSame('biz-1', $missingName['provider']['id'], 'other provider keys preserved');
226
227	$nonStringName = $this->normalizeProvider(['provider' => ['name' => 123], 'review_id' => 'x']);
228	$this->assertSame('', $nonStringName['provider']['name'], 'non-string name coerced to empty string');
229	}
230
231	public function test_non_array_post_data_is_passed_through_untouched(): void
232	{
233	// Scalar reviews are skipped by the Feed is_array() guards before
234	// construction; normalize must not choke on them either.
235	$this->assertSame('error-string', $this->normalizeProvider('error-string'));
236	$this->assertNull($this->normalizeProvider(null));
237	}
238
239	/**
240	* WPSA-63160 follow-up: the dedup key build (remove_duplicated_posts_list,
241	* 'json' branch, every front-end render) reads source['id'] + reviewer['name']
242	* off the RAW post. A scalar reviewer/source — which the is_array($single_review)
243	* -only cache guard lets through to store() — would fatal there on PHP 8 just
244	* like the provider did. normalize_review_shape now coerces them too.
245	*/
246	public function test_scalar_reviewer_and_source_are_coerced_to_arrays(): void
247	{
248	$out = $this->normalizeProvider([
249	'provider' => 'google',
250	'reviewer' => 'Jane Doe', // scalar — would fatal at reviewer['name']
251	'source' => 'ChIJscalarSource', // scalar — would fatal at source['id'] in the json branch
252	'rating' => 5,
253	]);
254
255	$this->assertIsArray($out['reviewer'], 'scalar reviewer coerced to array');
256	$this->assertSame('', $out['reviewer']['name'], 'reviewer name read key present + empty');
257	$this->assertSame('', $out['reviewer']['avatar'], 'reviewer avatar read key present');
258	$this->assertIsArray($out['source'], 'scalar source coerced to array');
259	$this->assertSame('', $out['source']['id'], 'source id read key present + empty');
260	$this->assertSame('', $out['source']['url'], 'source url read key present');
261
262	// The exact dedup key build that fatals pre-fix now runs clean.
263	$key = $out['source']['id'] . '-' . $out['rating'] . '-' . $out['reviewer']['name'] . '-' . $out['provider']['name'];
264	$this->assertSame('-5--google', $key);
265	}
266
267	public function test_healthy_reviewer_and_source_are_preserved(): void
268	{
269	$reviewer = ['name' => 'Jane', 'avatar' => 'https://x/a.png', 'first_name' => 'Jane'];
270	$source = ['id' => 'place-9', 'url' => 'https://example.test'];
271	$out = $this->normalizeProvider([
272	'provider' => ['name' => 'google'],
273	'reviewer' => $reviewer,
274	'source' => $source,
275	]);
276
277	$this->assertSame('Jane', $out['reviewer']['name'], 'healthy reviewer name untouched');
278	$this->assertSame('Jane', $out['reviewer']['first_name'], 'extra reviewer keys preserved');
279	$this->assertSame('place-9', $out['source']['id'], 'healthy source id untouched');
280	$this->assertSame('https://example.test', $out['source']['url'], 'healthy source url untouched');
281	}
282
283	public function test_array_reviewer_missing_name_gets_empty_string_name(): void
284	{
285	// reviewer present but missing the read keys (partial relay shape).
286	$out = $this->normalizeProvider(['reviewer' => ['first_name' => 'Jo'], 'source' => ['id' => 's1']]);
287	$this->assertSame('', $out['reviewer']['name'], 'missing reviewer name filled');
288	$this->assertSame('', $out['reviewer']['avatar'], 'missing reviewer avatar filled');
289	$this->assertSame('Jo', $out['reviewer']['first_name'], 'existing reviewer key preserved');
290	$this->assertSame('s1', $out['source']['id'], 'existing source id preserved');
291	$this->assertSame('', $out['source']['url'], 'missing source url filled');
292	}
293
294	/**
295	* Audit follow-up: the image containers (media / reviews_photos) are iterated
296	* by resize_images() + add_local_image_urls(). A scalar there fatals the
297	* foreach / string-offset write on PHP 8 (verified raw on 8.4). normalize now
298	* coerces a present non-array container to [] (element-level scalars are
299	* additionally guarded at the loop sites).
300	*/
301	public function test_scalar_media_and_reviews_photos_coerced_to_arrays(): void
302	{
303	$out = $this->normalizeProvider(['provider' => 'google', 'media' => 'oops', 'reviews_photos' => 'x']);
304	$this->assertSame([], $out['media'], 'scalar media coerced to []');
305	$this->assertSame([], $out['reviews_photos'], 'scalar reviews_photos coerced to []');
306	}
307
308	public function test_absent_image_containers_are_not_added(): void
309	{
310	// media/reviews_photos are optional — don't fabricate keys (would flip !empty checks).
311	$out = $this->normalizeProvider(['provider' => 'google']);
312	$this->assertArrayNotHasKey('media', $out, 'absent media stays absent');
313	$this->assertArrayNotHasKey('reviews_photos', $out, 'absent reviews_photos stays absent');
314	}
315
316	public function test_healthy_media_array_preserved(): void
317	{
318	$media = [['type' => 'image', 'url' => 'https://x/1.jpg']];
319	$out = $this->normalizeProvider(['media' => $media, 'provider' => ['name' => 'google']]);
320	$this->assertSame($media, $out['media'], 'well-formed media array left intact');
321	}
322
323	/**
324	* PR #482 Copilot: a non-string source['id'] must coerce safely — cast a scalar
325	* (numeric id), but turn an array/object into '' rather than (string)-casting it
326	* (which would emit an "Array to string conversion" notice — the opposite of the
327	* normalizer's no-warning goal).
328	*/
329	public function test_non_string_source_id_coerced_without_array_to_string(): void
330	{
331	$arrayId = $this->normalizeProvider(['source' => ['id' => ['nested' => 'x'], 'url' => 'u']]);
332	$this->assertSame('', $arrayId['source']['id'], 'array source id -> empty string (no Array-to-string)');
333
334	$numericId = $this->normalizeProvider(['source' => ['id' => 12345]]);
335	$this->assertSame('12345', $numericId['source']['id'], 'numeric source id cast to string');
336	}
337
338	/**
339	* End-to-end on the real Util::parse_single_review() — the store-path reader
340	* (PostAggregator::get_related_reviews, duplicate_collection). A scalar
341	* provider fatally crashed its $review['provider']['name'] read pre-fix;
342	* now it's normalized at the method entry.
343	*/
344	public function test_parse_single_review_survives_string_provider(): void
345	{
346	$review = [
347	'time' => '1700000000',
348	'rating' => 5,
349	'text' => 'Great service',
350	'reviewer' => ['name' => 'Jane Doe', 'avatar' => ''],
351	'provider' => 'google', // scalar slug — pre-fix this fatals at the provider read
352	'source' => ['id' => 'p1', 'url' => 'https://example.test'],
353	];
354
355	$out = Util::parse_single_review($review, 'p1', 'r1');
356
357	$this->assertSame('google', $out['provider']['name'], 'string provider normalized + parsed, no fatal');
358	$this->assertSame('Jane Doe', $out['reviewer']['name']);
359	}
360	}
361