Skip to content

Commit 697206e

Browse files
authored
Fix TypeError when editing queued job with invalid JSON payload (#468)
When invalid JSON is submitted via the data edit form, the JsonableBehavior threw a TypeError because json_decode returned null but the return type was array. Changes: - Update _fromJson() to throw InvalidArgumentException with a descriptive error message instead of TypeError - Catch the exception in data() action and show a flash error - Preserve the user's invalid input so they can fix it - Add data_string virtual property annotation to QueuedJob entity - Add test for invalid JSON handling - Fix PHPStan errors: handle nullable description() in example tasks
1 parent ba99af3 commit 697206e

4 files changed

Lines changed: 42 additions & 7 deletions

File tree

src/Controller/Admin/QueuedJobsController.php

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Cake\Http\Exception\NotFoundException;
99
use Cake\I18n\DateTime;
1010
use Cake\View\JsonView;
11+
use InvalidArgumentException;
1112
use Queue\Queue\TaskFinder;
1213
use RuntimeException;
1314

@@ -304,14 +305,21 @@ public function data(?int $id = null) {
304305
}
305306

306307
if ($this->request->is(['patch', 'post', 'put'])) {
307-
$queuedJob = $this->QueuedJobs->patchEntity($queuedJob, $this->request->getData());
308-
if ($this->QueuedJobs->save($queuedJob)) {
309-
$this->Flash->success(__d('queue', 'The queued job has been saved.'));
308+
try {
309+
$queuedJob = $this->QueuedJobs->patchEntity($queuedJob, $this->request->getData());
310+
if ($this->QueuedJobs->save($queuedJob)) {
311+
$this->Flash->success(__d('queue', 'The queued job has been saved.'));
310312

311-
return $this->redirect(['action' => 'view', $id]);
312-
}
313+
return $this->redirect(['action' => 'view', $id]);
314+
}
313315

314-
$this->Flash->error(__d('queue', 'The queued job could not be saved. Please try again.'));
316+
$this->Flash->error(__d('queue', 'The queued job could not be saved. Please try again.'));
317+
} catch (InvalidArgumentException $e) {
318+
$this->Flash->error($e->getMessage());
319+
320+
// Preserve the user's invalid input so they can fix it
321+
$queuedJob->data_string = $this->request->getData('data_string');
322+
}
315323
}
316324

317325
$this->set(compact('queuedJob'));

src/Model/Behavior/JsonableBehavior.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,10 +216,15 @@ public function _decode($val) {
216216
/**
217217
* @param string $val
218218
*
219+
* @throws \InvalidArgumentException
220+
*
219221
* @return array
220222
*/
221223
protected function _fromJson(string $val): array {
222-
$json = json_decode($val, true, JSON_THROW_ON_ERROR);
224+
$json = json_decode($val, true);
225+
if (!is_array($json)) {
226+
throw new InvalidArgumentException('Invalid JSON: ' . json_last_error_msg());
227+
}
223228

224229
return $json;
225230
}

src/Model/Entity/QueuedJob.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
* @property int $id
1010
* @property string $job_task
1111
* @property array|null $data
12+
* @property string|null $data_string Virtual property from JsonableBehavior
1213
* @property string|null $job_group
1314
* @property string|null $reference
1415
* @property \Cake\I18n\DateTime $created

tests/TestCase/Controller/Admin/QueuedJobsControllerTest.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,27 @@ public function testDataPost() {
133133
$this->assertSame($expected, $job->data);
134134
}
135135

136+
/**
137+
* @return void
138+
*/
139+
public function testDataPostInvalidJson(): void {
140+
$job = $this->createJob(['data' => '{"valid":"json"}']);
141+
142+
$this->enableRetainFlashMessages();
143+
$data = [
144+
'data_string' => 'not valid json {',
145+
];
146+
$this->post(['prefix' => 'Admin', 'plugin' => 'Queue', 'controller' => 'QueuedJobs', 'action' => 'data', $job->id], $data);
147+
148+
$this->assertResponseCode(200);
149+
$this->assertFlashMessage('Invalid JSON: Syntax error');
150+
151+
// Verify original data was not modified
152+
/** @var \Queue\Model\Entity\QueuedJob $job */
153+
$job = $this->fetchTable('Queue.QueuedJobs')->get($job->id);
154+
$this->assertSame('{"valid":"json"}', $job->data);
155+
}
156+
136157
/**
137158
* Test index method
138159
*

0 commit comments

Comments
 (0)