Repository navigation
Refuse a too-long public answer before saving it - #293
Conversation
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
PhilReinking
left a comment
There was a problem hiding this comment.
Changes at df99601, CI green. Must-fix inline: a list inside a list skips the new check and gets a 500, where main gave a 422. The rest holds on MariaDB, debug on and off: the exact limit saves, one character over gets the 422 and saves nothing, for a, ä and 😀. The limit is 36,740 bytes, or in debug 65,533 a, 10,922 ä, 5,461 😀. Choices, numbers and null still save. Scribe docs unchanged.
| public function validate(string $attribute, mixed $value, Closure $fail): void | ||
| { | ||
| // a block sends one answer, or a list of answers (checkbox, consent) | ||
| $answers = is_array($value) && ! has_string_keys($value) ? $value : [$value]; | ||
|
|
||
| foreach ($answers as $answer) { | ||
| // measured the way it is stored: encrypted, or as JSON in debug mode | ||
| if (is_array($answer) && strlen(FormSessionResponse::toStored($answer['payload'] ?? null)) > FormSessionResponse::VALUE_MAX_BYTES) { | ||
| $fail('This answer is too long. Please shorten it and send it again.'); | ||
|
|
||
| return; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
A list inside a list skips this check and gets a 500 again. FormBlock::submit walks lists at any depth (app/Models/FormBlock.php:211), but this rule looks only one level deep. For [[{actionId, payload: 70,000 × "a"}]], $answer['payload'] is null, so the check passes and MariaDB throws SQLSTATE 22001. On main, the 30,000 cap measured the whole value, so the same request got a 422. Only a hand-made API call sends this, and nothing is saved, but it is the 500 this PR removes.
Walk the answer the same way the save does:
| public function validate(string $attribute, mixed $value, Closure $fail): void | |
| { | |
| // a block sends one answer, or a list of answers (checkbox, consent) | |
| $answers = is_array($value) && ! has_string_keys($value) ? $value : [$value]; | |
| foreach ($answers as $answer) { | |
| // measured the way it is stored: encrypted, or as JSON in debug mode | |
| if (is_array($answer) && strlen(FormSessionResponse::toStored($answer['payload'] ?? null)) > FormSessionResponse::VALUE_MAX_BYTES) { | |
| $fail('This answer is too long. Please shorten it and send it again.'); | |
| return; | |
| } | |
| } | |
| } | |
| public function validate(string $attribute, mixed $value, Closure $fail): void | |
| { | |
| if ($this->tooLong($value)) { | |
| $fail('This answer is too long. Please shorten it and send it again.'); | |
| } | |
| } | |
| // walks the answer the way FormBlock::submit stores it: a list holds more answers (checkbox, consent), at any depth | |
| private function tooLong(mixed $value): bool | |
| { | |
| if (! is_array($value)) { | |
| return false; | |
| } | |
| if (! has_string_keys($value)) { | |
| return collect($value)->contains(fn ($answer) => $this->tooLong($answer)); | |
| } | |
| // measured the way it is stored: encrypted, or as JSON in debug mode | |
| return strlen(FormSessionResponse::toStored($value['payload'] ?? null)) > FormSessionResponse::VALUE_MAX_BYTES; | |
| } |
With this, that request gets the 422. FormSessionTest.php plus my checks (checkbox lists, numbers, null, a 33,000-byte answer) pass: 38 tests on MariaDB, debug on and off.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
PhilReinking
left a comment
There was a problem hiding this comment.
Changes at cc75cd9, CI green. The rule now walks lists like FormBlock::submit. A long answer one list down or four lists down gets the 422 and saves nothing. Short answers four lists down all save, and an empty list passes. 43 tests pass on MariaDB, debug on and off. The new test fails on the old rule. Must-fix inline: the changelog's 36,000 holds only for plain letters.
| - Browser Back now leaves a form's Settings page. Before, every tab you opened added a history step, so Back only went through the tabs. A link to a Settings tab still opens that tab. (#287) | ||
| - A new long text question now shows in your form and in the editor preview, also before you set any options for it. Before, it failed to show. (#286) | ||
| - A long text question with an empty "Max characters" now has no limit, also when you open it again. Before, the editor showed 500 and saved it back. New long text questions still start at 500. (#291) | ||
| - Long answers are now accepted up to what the database can hold (about 36,000 characters), and a longer one is refused with a clear message and nothing is saved. Before, answers over 30,000 characters were refused with a technical message. (#293) |
There was a problem hiding this comment.
"About 36,000 characters" holds only for plain letters. The limit is in bytes. In production, 36,740 plain letters fit, but only 18,370 umlauts or accented letters, 12,246 Chinese characters or 9,185 emoji. The old cap was in bytes too, so it was about 30,000 plain letters or 15,000 umlauts. Input ships French, Polish, Ukrainian and Chinese translations, so the line should say the limit is lower for those:
| - Long answers are now accepted up to what the database can hold (about 36,000 characters), and a longer one is refused with a clear message and nothing is saved. Before, answers over 30,000 characters were refused with a technical message. (#293) | |
| - 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) |
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
PhilReinking
left a comment
There was a problem hiding this comment.
OK at 5bf280a, CI green. Since cc75cd9 only CHANGELOG.md changed. The #293 line now matches the suggested wording word for word. It is the last line of Fixed under Unreleased, just above Security. Against main, the changelog is +1/-0. The code is unchanged since cc75cd9, where 43 tests passed on MariaDB, debug on and off.
The submit check now measures an answer the way it is stored (encrypted, or JSON in debug mode) and refuses it with a 422 and a clear message when it is over the 65,535 bytes of the
valuecolumn. It runs before anything is written, so one too-long answer saves none of the others. Before, a flat 30,000-byte cap ran, and in debug mode an answer with umlauts or emoji could still end in a 500 on MariaDB.Part of BITS-347 (criterion 4; the rest is in the frontend PR).
Tested: Pest on MariaDB and SQLite, full suite green; the new tests fail without the fix.
Not tested: the public form in a browser. It shows only the fixed "submit failed" text, not the server message (
resources/js/stores/conversation.ts:563).