Skip to content

Commit 4815abe

Browse files
btrybtry
committed
fix(radiosfield,checkboxesfield,glpiselectfield,requesttypefield,urgencyfield): validate default value before saving
1 parent fc0ea0a commit 4815abe

11 files changed

+330
-69
lines changed

‎inc/abstractfield.class.php

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,13 @@ public function getLabel() {
160160

161161
/**
162162
* Gets the available values for the field
163+
* @param array $values values to parse. If null, the values are ised from the instance of the question
163164
* @return array available values
164165
*/
165-
public function getAvailableValues() {
166-
$values = json_decode($this->question->fields['values']);
166+
public function getAvailableValues(array $values = null): array {
167+
if ($values === null) {
168+
$values = json_decode($this->question->fields['values']);
169+
}
167170
$tab_values = [];
168171
foreach ($values as $value) {
169172
$trimmedValue = trim($value);
@@ -179,13 +182,11 @@ public function isRequired(): bool {
179182
}
180183

181184
/**
182-
* trim values separated by \r\n
185+
* trim and explode values separated by \r\n
183186
* @param string $value a value or default value
184-
* @return string
187+
* @return array
185188
*/
186-
protected function trimValue($value) {
187-
global $DB;
188-
189+
protected function trimValue($value): array {
189190
$value = explode('\r\n', $value);
190191
// input has escpaed single quotes
191192
$value = Toolbox::stripslashes_deep($value);
@@ -199,6 +200,7 @@ function ($value) {
199200
$value
200201
);
201202

203+
return $value;
202204
return $DB->escape(json_encode($value, JSON_UNESCAPED_UNICODE));
203205
}
204206

‎inc/field/checkboxesfield.class.php

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -249,22 +249,33 @@ public function isValidValue($value): bool {
249249
}
250250

251251
public function prepareQuestionInputForSave($input) {
252+
global $DB;
253+
252254
if (!isset($input['values']) || empty($input['values'])) {
253255
Session::addMessageAfterRedirect(
254-
__('The field value is required:', 'formcreator') . ' ' . $input['name'],
256+
__('The field value is required.', 'formcreator'),
255257
false,
256258
ERROR
257259
);
258260
return [];
259261
}
260262

261-
// trim values
262-
$input['values'] = $this->trimValue($input['values']);
263-
264-
if (isset($input['default_values'])) {
263+
$values = $this->trimValue($input['values']);
264+
$defaultValues = $this->trimValue($input['default_values'] ?? '');
265+
if (count($defaultValues) > 0) {
265266
// trim values
266-
$input['default_values'] = $this->trimValue($input['default_values']);
267+
$validDefaultValues = array_intersect($this->getAvailableValues($values), $defaultValues);
268+
if (count($validDefaultValues) != (count($defaultValues))) {
269+
Session::addMessageAfterRedirect(
270+
__('The default values are not in the list of available values.', 'formcreator'),
271+
false,
272+
ERROR
273+
);
274+
return [];
275+
}
276+
$input['default_values'] = $DB->escape(json_encode($defaultValues, JSON_UNESCAPED_UNICODE));
267277
}
278+
$input['values'] = $DB->escape(json_encode($values, JSON_UNESCAPED_UNICODE));
268279

269280
return $input;
270281
}

‎inc/field/glpiselectfield.class.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ public function showForm(array $options): void {
7272
}
7373
$this->question->fields['default_values'] = Html::entities_deep($this->question->fields['default_values']);
7474
$this->deserializeValue($this->question->fields['default_values']);
75+
$this->question->fields['_default_values'] = $this->value;
7576

7677
TemplateRenderer::getInstance()->display($template, [
7778
'item' => $this->question,
@@ -93,6 +94,8 @@ public function isValidValue($value): bool {
9394
}
9495

9596
public function prepareQuestionInputForSave($input) {
97+
global $DB;
98+
9699
if (!isset($input['itemtype']) || empty($input['itemtype'])) {
97100
Session::addMessageAfterRedirect(
98101
__('The field value is required:', 'formcreator') . ' ' . $input['name'],
@@ -122,7 +125,7 @@ public function prepareQuestionInputForSave($input) {
122125
unset($input['show_tree_depth']);
123126
unset($input['selectable_tree_root']);
124127

125-
$input['values'] = json_encode($input['values']);
128+
$input['values'] = $DB->escape(json_encode($input['values'], JSON_UNESCAPED_UNICODE));
126129

127130
return $input;
128131
}
@@ -131,7 +134,7 @@ public static function canRequire(): bool {
131134
return true;
132135
}
133136

134-
public function getAvailableValues(): array {
137+
public function getAvailableValues(array $values = null): array {
135138
return [];
136139
}
137140

‎inc/field/radiosfield.class.php

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public function isPrerequisites(): bool {
4949
public function showForm(array $options): void {
5050
$template = '@formcreator/field/' . $this->question->fields['fieldtype'] . 'field.html.twig';
5151

52-
$this->question->fields['values'] = json_decode($this->question->fields['values']);
52+
$this->question->fields['values'] = json_decode($this->question->fields['values']);
5353
$this->question->fields['values'] = is_array($this->question->fields['values']) ? $this->question->fields['values'] : [];
5454
$this->question->fields['values'] = implode("\r\n", $this->question->fields['values']);
5555
$this->deserializeValue($this->question->fields['default_values']);
@@ -109,18 +109,41 @@ public static function getName(): string {
109109
}
110110

111111
public function prepareQuestionInputForSave($input) {
112+
global $DB;
113+
112114
if (!isset($input['values']) || empty($input['values'])) {
113115
Session::addMessageAfterRedirect(
114-
__('The field value is required:', 'formcreator') . ' ' . $input['name'],
116+
__('The field value is required.', 'formcreator'),
115117
false,
116118
ERROR
117119
);
118120
return [];
119121
}
120122

121-
// trim values
122-
$input['values'] = $this->trimValue($input['values']);
123-
$input['default_values'] = trim($input['default_values']);
123+
// trim values (actually there is only one value then no \r\n expected)
124+
$defaultValues = $this->trimValue($input['default_values'] ?? '');
125+
if (count($defaultValues) > 1) {
126+
Session::addMessageAfterRedirect(
127+
__('Only one default value is allowed.', 'formcreator'),
128+
false,
129+
ERROR
130+
);
131+
return [];
132+
}
133+
$values = $this->trimValue($input['values']);
134+
if (count($defaultValues) > 0) {
135+
$validDefaultValues = array_intersect($this->getAvailableValues($values), $defaultValues);
136+
if (count($validDefaultValues) != count($defaultValues)) {
137+
Session::addMessageAfterRedirect(
138+
__('The default value is not in the list of available values.', 'formcreator'),
139+
false,
140+
ERROR
141+
);
142+
return [];
143+
}
144+
}
145+
$input['values'] = $DB->escape(json_encode($values, JSON_UNESCAPED_UNICODE));
146+
$input['default_values'] = array_pop($defaultValues);
124147

125148
return $input;
126149
}

‎inc/field/requesttypefield.class.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public static function canRequire(): bool {
111111
return true;
112112
}
113113

114-
public function getAvailableValues() {
114+
public function getAvailableValues(array $values = null): array {
115115
return Ticket::getTypes();
116116
}
117117

‎inc/field/urgencyfield.class.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ public static function canRequire(): bool {
114114
return true;
115115
}
116116

117-
public function getAvailableValues() {
117+
public function getAvailableValues(array $values = null): array {
118118
return [
119119
5 => _x('urgency', 'Very high'),
120120
4 => _x('urgency', 'High'),

‎inc/formanswer.class.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1368,6 +1368,8 @@ public function parseTags(string $content, PluginFormcreatorTargetInterface $tar
13681368
$content = Sanitizer::sanitize($content);
13691369
}
13701370

1371+
$content = $this->parseExtraTags($content, $target, $richText);
1372+
13711373
$hook_data = Plugin::doHookFunction('formcreator_parse_extra_tags', [
13721374
'formanswer' => $this,
13731375
'content' => $content,
@@ -1378,6 +1380,12 @@ public function parseTags(string $content, PluginFormcreatorTargetInterface $tar
13781380
return $hook_data['content'];
13791381
}
13801382

1383+
protected function parseExtraTags(string $content, PluginFormcreatorTargetInterface $target = null, $richText = false): string {
1384+
$content = str_replace('##answer_id##', $this->getField('id'), $content);
1385+
1386+
return $content;
1387+
}
1388+
13811389
/**
13821390
* Validates answers of a form
13831391
*

‎templates/field/glpiselectfield.html.twig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@
7272
{{ fields.dropdownField(
7373
item.fields['itemtype'],
7474
'default_values',
75-
default_values,
75+
item.fields['_default_values'],
7676
__('Default values'), {
7777
label_class: 'col-xxl-4',
7878
input_class: 'col-xxl-8',

‎tests/3-unit/GlpiPlugin/Formcreator/Field/CheckboxesField.php

Lines changed: 138 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -234,41 +234,170 @@ public function testDeserializeValue($value, $expected) {
234234
$this->string($output)->isEqualTo(implode(', ', $expected));
235235
}
236236

237-
public function testPrepareQuestionInputForSave() {
237+
public function providerPrepareQuestionInputForSave() {
238+
global $DB;
239+
238240
$question = $this->getQuestion([
239241
'fieldtype' => 'checkboxes',
240242
'name' => 'question',
241243
'required' => '0',
242-
'default_values' => json_encode(['1', '2', '3', '5', '6']),
243-
'values' => json_encode(['1', '2', '3', '4', '5', '6']),
244+
'values' => '1\r\n2\r\n3\r\n4\r\n5\r\n6',
245+
'default_values' => '1\r\n2\r\n3\r\n5\r\n6',
244246
'order' => '1',
245247
'show_rule' => \PluginFormcreatorCondition::SHOW_RULE_ALWAYS,
246248
'range_min' => 3,
247249
'range_max' => 4,
248250
]);
251+
252+
yield [
253+
'field' => $this->newTestedInstance($question),
254+
'input' => [
255+
'values' => "",
256+
'name' => 'foo',
257+
],
258+
'expected' => [],
259+
'message' => 'The field value is required.',
260+
];
261+
262+
yield [
263+
'field' => $this->newTestedInstance($question),
264+
'input' => [
265+
'values' => 'éè\r\nsomething else',
266+
'default_values' => 'éè',
267+
],
268+
'expected' => [
269+
'values' => '[\"éè\",\"something else\"]',
270+
'default_values' => '[\"éè\"]',
271+
],
272+
'message' => '',
273+
];
274+
275+
yield [
276+
'field' => $this->newTestedInstance($question),
277+
'input' => [
278+
'values' => ' something \r\n something else ',
279+
'default_values' => ' something ',
280+
],
281+
'expected' => [
282+
'values' => '[\"something\",\"something else\"]',
283+
'default_values' => '[\"something\"]',
284+
],
285+
'message' => '',
286+
];
287+
288+
yield 'no default value' => [
289+
'field' => $this->newTestedInstance($question),
290+
'input' => [
291+
'values' => 'a\r\nb\r\nc',
292+
'name' => 'foo',
293+
'default_values' => ''
294+
],
295+
'expected' => [
296+
'values' => $DB->escape('["a","b","c"]'),
297+
'name' => 'foo',
298+
'default_values' => '',
299+
],
300+
'message' => '',
301+
];
302+
303+
yield 'several default values' => [
304+
'field' => $this->newTestedInstance($question),
305+
'input' => [
306+
'values' => 'a\r\nb\r\nc',
307+
'name' => 'foo',
308+
'default_values' => 'a\r\n\b'
309+
],
310+
'expected' => [
311+
'values' => $DB->escape('["a","b","c"]'),
312+
'name' => 'foo',
313+
'default_values' => $DB->escape('["a","b"]'),
314+
],
315+
'message' => '',
316+
];
317+
318+
yield 'one default value' => [
319+
'field' => $this->newTestedInstance($question),
320+
'input' => [
321+
'values' => 'a\r\nb\r\nc',
322+
'name' => 'foo',
323+
'default_values' => 'b'
324+
],
325+
'expected' => [
326+
'values' => $DB->escape('["a","b","c"]'),
327+
'name' => 'foo',
328+
'default_values' => $DB->escape('["b"]'),
329+
],
330+
'message' => '',
331+
];
332+
333+
yield 'invalid default value' => [
334+
'field' => $this->newTestedInstance($question),
335+
'input' => [
336+
'values' => 'a\r\nb\r\nc',
337+
'name' => 'foo',
338+
'default_values' => 'z'
339+
],
340+
'expected' => [],
341+
'message' => 'The default values are not in the list of available values.',
342+
];
343+
}
344+
345+
/**
346+
* @dataProvider providerPrepareQuestionInputForSave
347+
*
348+
* @return void
349+
*/
350+
public function testPrepareQuestionInputForSave($field, $input, $expected, $message) {
351+
352+
// Clean error messages
353+
$_SESSION['MESSAGE_AFTER_REDIRECT'] = [];
354+
355+
$output = $field->prepareQuestionInputForSave($input);
356+
if ($expected === false || is_array($expected) && count($expected) == 0) {
357+
$this->array($output)->hasSize(0);
358+
$this->sessionHasMessage($message, ERROR);
359+
//End of test on expected failure
360+
return;
361+
}
362+
363+
$this->array($output)->isEqualTo($expected);
364+
365+
return;
366+
367+
$question = $this->getQuestion([
368+
'fieldtype' => 'checkboxes',
369+
'name' => 'question',
370+
'required' => '0',
371+
'default_values' => json_encode(['1', '2', '3', '5', '6']),
372+
'values' => json_encode(['1', '2', '3', '4', '5', '6']),
373+
'order' => '1',
374+
'show_rule' => \PluginFormcreatorCondition::SHOW_RULE_ALWAYS,
375+
'range_min' => 3,
376+
'range_max' => 4,
377+
]);
249378
$fieldInstance = $this->newTestedInstance($question);
250379

251380
// Test a value is mandatory
252381
$input = [
253-
'values' => "",
254-
'name' => 'foo',
382+
'values' => "",
383+
'name' => 'foo',
255384
];
256385
$out = $fieldInstance->prepareQuestionInputForSave($input);
257386
$this->integer(count($out))->isEqualTo(0);
258387

259388
// Test accented chars are kept
260389
$input = [
261-
'values' => 'éè\r\nsomething else',
262-
'default_values' => 'éè',
390+
'values' => 'éè\r\nsomething else',
391+
'default_values' => 'éè',
263392
];
264393
$out = $fieldInstance->prepareQuestionInputForSave($input);
265394
$this->string($out['values'])->isEqualTo('[\"éè\",\"something else\"]');
266395
$this->string($out['default_values'])->isEqualTo('[\"éè\"]');
267396

268397
// Test values are trimmed
269398
$input = [
270-
'values' => ' something \r\n something else ',
271-
'default_values' => ' something ',
399+
'values' => ' something \r\n something else ',
400+
'default_values' => ' something ',
272401
];
273402
$out = $fieldInstance->prepareQuestionInputForSave($input);
274403
$this->string($out['values'])->isEqualTo('[\"something\",\"something else\"]');

0 commit comments

Comments
 (0)