diff --git a/CHANGELOG.md b/CHANGELOG.md index 448a15af..3afc884d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,6 +74,7 @@ Lines marked **Upgrade step** need you to act after updating. - A webhook answer that isn't valid UTF-8 text is now saved and shows on the submissions page, with each unreadable character as "?". Before, it wasn't saved and the webhook was called up to 5 times. With `QUEUE_CONNECTION=sync`, the submission itself failed with a server error. (#284) - Opening a question in the editor no longer saves its answer settings, so "Saving…" does not show up for nothing. They save only when you change a setting. (#288) - When a save fails, the red bar with Retry no longer makes the editor taller than your window. The page stays as tall as the window, so your last question and the end screen stay in reach. (#290) +- Long answers are now accepted up to what the database can hold (about 36,000 plain letters, fewer with accents, emoji or non-Latin script), and a longer one is refused with a clear message and nothing is saved. Before, the limit was lower (about 30,000 plain letters) and the refusal was a technical message. (#293) ### Security diff --git a/app/Http/Controllers/Api/FormSubmitController.php b/app/Http/Controllers/Api/FormSubmitController.php index 584364e5..cd438781 100644 --- a/app/Http/Controllers/Api/FormSubmitController.php +++ b/app/Http/Controllers/Api/FormSubmitController.php @@ -6,7 +6,7 @@ use App\Http\Controllers\Controller; use App\Models\Form; use App\Models\FormSession; -use App\Rules\MaxSize; +use App\Rules\AnswerFitsColumn; use Illuminate\Http\Request; use Knuckles\Scribe\Attributes\BodyParam; use Knuckles\Scribe\Attributes\Group; @@ -41,8 +41,7 @@ public function __invoke(Request $request, Form $form) 'token' => 'required|string', 'is_uploading' => 'boolean', 'payload' => 'array|nullable|max:'.$form->formBlocks()->count(), - // an answer is stored encrypted in a TEXT column (64 KB), encryption almost doubles it - 'payload.*' => [new MaxSize(30000)], + 'payload.*' => [new AnswerFitsColumn], ]); // a preview stores and sends nothing, see CreateFormSessionController diff --git a/app/Models/FormSessionResponse.php b/app/Models/FormSessionResponse.php index 48179652..92f28775 100644 --- a/app/Models/FormSessionResponse.php +++ b/app/Models/FormSessionResponse.php @@ -14,6 +14,9 @@ class FormSessionResponse extends Model { use HasFactory; + // the `value` column is a TEXT + public const VALUE_MAX_BYTES = 65535; + protected $guarded = []; protected $casts = [ @@ -50,7 +53,12 @@ public function formSessionUploads() public function setValueAttribute($new) { - $this->attributes['value'] = config('app.debug') ? json_encode($new) : encrypt($new); + $this->attributes['value'] = self::toStored($new); + } + + public static function toStored($value): string + { + return config('app.debug') ? json_encode($value) : encrypt($value); } public function getValueAttribute() diff --git a/app/Rules/AnswerFitsColumn.php b/app/Rules/AnswerFitsColumn.php new file mode 100644 index 00000000..74c320eb --- /dev/null +++ b/app/Rules/AnswerFitsColumn.php @@ -0,0 +1,32 @@ +tooLong($value)) { + $fail('This answer is too long. Please shorten it and send it again.'); + } + } + + // walks lists at any depth, the way FormBlock::submit does + private function tooLong(mixed $value): bool + { + if (! is_array($value)) { + return false; + } + + if (has_string_keys($value)) { + // measured the way it is stored: encrypted, or as JSON in debug mode + return strlen(FormSessionResponse::toStored($value['payload'] ?? null)) > FormSessionResponse::VALUE_MAX_BYTES; + } + + return collect($value)->contains(fn ($answer) => $this->tooLong($answer)); + } +} diff --git a/tests/Feature/FormSessionTest.php b/tests/Feature/FormSessionTest.php index 8071d89c..d4c7a282 100644 --- a/tests/Feature/FormSessionTest.php +++ b/tests/Feature/FormSessionTest.php @@ -338,23 +338,73 @@ 'many short parameters' => [collect(range(1, 600))->mapWithKeys(fn ($i) => ["p$i" => 'x'])->all()], ]); -test('an answer over the size cap is rejected and not stored', function ($template) { +test('an answer at the size cap is stored, one character over is rejected with a clear message', function ($template, $char) { $form = Form::factory()->create(); $form->applyTemplate($template); $session = FormSession::factory()->create(['form_id' => $form->id]); $block = $form->formBlocks[0]; + // the longest answer of this character that still fits the TEXT column once stored + $fits = fn (int $count) => strlen(FormSessionResponse::toStored(str_repeat($char, $count))) <= 65535; + [$low, $high] = [1, 65535]; + while ($low < $high) { + $middle = intdiv($low + $high + 1, 2); + $fits($middle) ? $low = $middle : $high = $middle - 1; + } + $atCap = str_repeat($char, $low); + + expect($fits($low + 1))->toBeFalse(); + $this->json('POST', route('api.public.forms.submit', ['form' => $form->uuid]), [ 'token' => $session->token, - 'payload' => $block->getSubmitPayload(str_repeat('a', 30000)), - ])->assertStatus(422)->assertJsonValidationErrors('payload.'.$block->uuid); + 'payload' => $block->getSubmitPayload($atCap.$char), + ]) + ->assertStatus(422) + ->assertJsonValidationErrors('payload.'.$block->uuid) + ->assertJsonPath('message', 'This answer is too long. Please shorten it and send it again.'); expect(FormSessionResponse::count())->toBe(0); $this->json('POST', route('api.public.forms.submit', ['form' => $form->uuid]), [ 'token' => $session->token, - 'payload' => $block->getSubmitPayload(str_repeat('a', 29000)), + 'payload' => $block->getSubmitPayload($atCap), ])->assertOk(); + + expect(FormSessionResponse::count())->toBe(1); +})->with('templates')->with(['one byte per character' => 'a', 'two bytes per character' => 'ä', 'four bytes per character' => '😀']); + +test('one answer over the size cap saves none of the other answers', function ($template) { + $form = Form::factory()->create(); + $form->applyTemplate($template); + $session = FormSession::factory()->create(['form_id' => $form->id]); + [$first, , , $last] = $form->formBlocks; + + $this->json('POST', route('api.public.forms.submit', ['form' => $form->uuid]), [ + 'token' => $session->token, + 'payload' => [ + ...$first->getSubmitPayload('froggy@mailfrog.com'), + ...$last->getSubmitPayload(str_repeat('a', 65535)), + ], + ]) + ->assertStatus(422) + ->assertJsonPath('message', 'This answer is too long. Please shorten it and send it again.'); + + expect(FormSessionResponse::count())->toBe(0); +})->with('templates'); + +test('a too-long answer inside nested lists is rejected and not stored', function ($template) { + $form = Form::factory()->create(); + $form->applyTemplate($template); + $session = FormSession::factory()->create(['form_id' => $form->id]); + $block = $form->formBlocks[0]; + $answer = $block->getSubmitPayload(str_repeat('a', 70000))[$block->uuid]; + + $this->json('POST', route('api.public.forms.submit', ['form' => $form->uuid]), [ + 'token' => $session->token, + 'payload' => [$block->uuid => [[$answer]]], + ])->assertStatus(422)->assertJsonValidationErrors('payload.'.$block->uuid); + + expect(FormSessionResponse::count())->toBe(0); })->with('templates'); test('more answers than the form has blocks are rejected', function ($template) {