[GLPI10.0.0-rc2] HTML tags escaping in textarea

[VladoTTX]

Describe the bug Firstly there seems to be general issue with GLPI10 where it is not able to display correctly HTML content which contains both types of escaping (< and >) and (< and >), Content needs to contain only one type of it.

Formcreator escaping of Textarea produces content which mixes both types of escaping and this does not work well with GLPI10 rendering. Also looks like escaping happens two times (with latest version of plugin). First time characters are escaped using < and >. In second pass these characters are escaped again so it becomes <

Also new line in textarea produces "rn" characters in output (looks like escaping of \r\n went wrong). Previously in 9.5.x new line in text area produced <p>line1</p><p>line2</p>

To Reproduce Steps to reproduce the behavior:

1. Create form which includes Textarea with target Ticket which incudes this field
2. Submit answer to form and check resulting ticket

Expected behavior Escaping should happen only once for the content and it should contain only one type of escaping. Either (< and >) or (< and >)

Screenshots Example from latest support/2.13.0 version:

The content field of latest support/2.13.0 version: <div><h1>Form data</h1><h2>Section One</h2><div><b>1) Q1 : </b>answer1</div><div><b>2) Area1 : </b><p>normal line</p>rn<p><strong>bold line</strong></p>rn<p><em>italic line</em></p>rn<ol>rn<li>list item1</li>rn<li>item2</li>rn<li>item3</li>rn</ol></div></div>

Example from 2.13.0-alpha3:

The content field of 2.13.0-alpha3: <div><h1>Form data</h1><h2>Incident form #1</h2><div><b>1) Title : </b>TEST fullform</div><div><b>2) Description : </b><p>full line1</p>rn<p><strong>bold line2</strong></p>rn<p><em>italic line3</em></p></div></div>

GLPI / Plugins (please complete the following information):

• GLPI version 10.0.0-rc2
• Formcreator various versions: latest support/2.13.0 and 2.13.0-alpha.3 (both have different issues)

[VladoTTX]

Checked on problem with rn showing in HTML seems that in 9.5.x content of fields excluded all \r\n in formcreatorfield*

Now in GLPI10 TextArea receives input which includes also \r\n in the content of formcreatorfield*. Example from GLPI 9.5.7: <p>LINE1</p><p><strong>LINE2</strong></p><p><em>LINE3</em></p>

Example from GLPI 10.0.0-rc2: <p>L1</p>\r\n<p><strong>L2</strong></p>\r\n<p><em>L3</em></p>

Result is that TextField::parseAnswerValues() strips the slashes and leaves rn afterwards: $this->value = Toolbox::stripslashes_deep($input[$key]);

Not sure about correct fix while checking in GLPI10 in DB, now content fields commonly contain also new lines (ITILFollowups and Tickets for example).

So now in GLPI10 slashes and new lines should not be stripped before saving in DB?

[btry]

Hi

It may be necessary to use the methods found in Toolbox\Sanitizer. Some other places were already updated after finding similar problems.

[VladoTTX]

Yeah, probably previously input was escaped by slashes and now it is using HTML escaping. This is new behavior that in multiline text there are new lines coded as \r\n. I am not sure what it needs to be, whether there could be new lines also in answers DB fields or should be removed/recoded?

Also this sanitizing looks like is causing the issues in PluginFormcreatorFormAnswer::parseTags() getValueForTargetText is returning answer coded using < > while remining $content is coded as < and >:

         if (PluginFormcreatorFields::isVisible($question, $this->questionFields)) {
            $name  = $question->fields['name'];
            $value = $this->questionFields[$questionId]->getValueForTargetText($domain, $richText);
         }

And at the end and last pass of encoding recodes everything once again. Where it does not work well when both escaping types are mixed < > and < > are used in $content. It recodes < to < and > to >

      if ($richText) {
         // convert sanitization from old style GLPI ( up to 9.5 to modern style)
         $content = Sanitizer::unsanitize($content);
         $content = Sanitizer::sanitize($content);
      }

Probably everything should be coded same way everywhere so replace Html::entities_deep with Sanitizer::sanitize in field classes getValueForTargetText() ?

[btry]

Hi

First, I would advise you that a PR should be merged soon (pending for review by a pair)

https://github.com/pluginsGLPI/formcreator/pull/2622

It implement twig when editing a question, for all types, and the change is big enough to impact your issue.

I'm starting investigation based on this branch.

Currently, I'm having the following changes to fix some encoding or rendering problems with default value.

diff --git a/inc/field/textareafield.class.php b/inc/field/textareafield.class.php
index f1827c0b..ebd17b4d 100644
--- a/inc/field/textareafield.class.php
+++ b/inc/field/textareafield.class.php
@@ -52,7 +52,7 @@ class TextareaField extends TextField

    public function showForm(array $options): void {
       $template = '@formcreator/field/' . $this->question->fields['fieldtype'] . 'field.html.twig';
-      $this->question->fields['default_values'] = Html::entities_deep($this->question->fields['default_values']);
+      // $this->question->fields['default_values'] = Html::entities_deep($this->question->fields['default_values']);
       $this->deserializeValue($this->question->fields['default_values']);
       $parameters = $this->getParameters();
       TemplateRenderer::getInstance()->display($template, [
@@ -183,7 +183,7 @@ class TextareaField extends TextField
       if (!$success) {
          return [];
       }
-      $this->value = Toolbox::stripslashes_deep(str_replace('\r\n', "\r\n", $input['default_values']));
+      // $this->value = Toolbox::stripslashes_deep(str_replace('\r\n', "\r\n", $input['default_values']));

       // Handle uploads
       $key = 'formcreator_field_' . $this->question->getID();

[btry]

I'm reviewing the escaping strategy globally on all question types. I think that next week you will have a version which works with textareas (solved on my side).

[VladoTTX]

Thanks @btry! I'll check. IMHO maybe also fixing Sanitizer::decodeHtmlSpecialChars could be a way. It does not convert LEGACY_CHARS_MAPPING if it detect CHARS_MAPPING (eg. mixed encoding as in formcreator case).

[btry]

In my change I removed most intermediate html escaping occurring while generating the content of a ticket. With GLPI 9.x I had to manage merges between the target template and the answers. This was complicated, and now it seems to not be necessary. I'll push next week the current state of my experiments. It seems to run well, but unit tests now need to be updated.

[btry]

Hi

I have good results with rework on escaping done in #2622. There are changes in all question allowing one or several items (radios, checkbowes, select, multiselect). Textarea should no longer produce visible HTML tags.

Please test it. I add here a build of this branch as it is right now. glpi-formcreator-2.13.0-alpha.3-dev.zip

[VladoTTX]

Thanks @btry! Tested provided version and it looks fine now. Ticket content is now correctly formatted without above issues. I've tested escaping of ' " \ and also looks OK. I've checked also how it looks in DB. These are findings (let me know whether all are expected):

1. Ticket content is escaped now completely with new tags &# and contain new lines
2. Answers are not escaped and contain non-escaped HTML tags and new lines

Another unrelated thing.. I noticed with latest versions of plugin (please let know whether this is expected):

1. By default no forms are displayed in formlist.php, you need to explicitly select category
2. When category is selected, also forms which have is_active = 0 are displayed

[btry]

By default no forms are displayed in formlist.php, you need to explicitly select category

yes, this is intentional

When category is selected, also forms which have is_active = 0 are displayed

There is a regression here. Thanks for the report, i'll fix it.

[VladoTTX]

Thanks, few more regressions I have noticed let me know whether I should open separate issues. This one seems to affect all versions I tested:

• After the form is submitted (Send button), now formcreator does not take into account whether field is part of hidden section. It still requires it even when whole section is hidden.

These seems to affect just this latest versions (2.13.0-alpha+):

1. On Sections and questions you can fill "Condition to show the section" just after they are saved. On new one, it does not react to change of dropdown selection to "Hidden unless" and it does not allow you to enter condition
2. After creating new section and saving it, it is not displayed on screen but just Questions of previous section are duplicated. See picture below.

Q3 was duplicated, when new section should have been added: After refresh it is displayed:

[btry]

When category is selected, also forms which have is_active = 0 are displayed

There is a regression here. Thanks for the report, i'll fix it.

Should be fixed with the last commir of the branch 80946804b054d23c2a6993c8e55cf65990dfbd58

[btry]

On Sections and questions you can fill "Condition to show the section" just after they are saved. On new one, it does not react to change of dropdown selection to "Hidden unless" and it does not allow you to enter condition

Got it, and fixed as well

[btry]

After the form is submitted (Send button), now formcreator does not take into account whether field is part of hidden section. It still requires it even when whole section is hidden.

fixed

[btry]

After creating new section and saving it, it is not displayed on screen but just Questions of previous section are duplicated. See picture below.

fixed

[VladoTTX]

Nice, thanks much! Will test once you commit the changes

[btry]

Hi

All fixes are already commited in the branch

[VladoTTX]

Hi

All fixes are already commited in the branch

Hmm, I don't see them in https://github.com/pluginsGLPI/formcreator/commit/80946804b054d23c2a6993c8e55cf65990dfbd58 or https://github.com/pluginsGLPI/formcreator/pull/2622 Anyway thanks for looking..

[amatteo78]

same on my side, I see ticket with html tag, GLPI 10.0.0-rc2 + 2.13.0-alpha.3

[btry]

I'll provide an up to date archive, based on a branch under review, to be merged soon. The issue should be fixed with this revision.

[btry]

@amatteo78 please test this build. It is based from #2622

glpi-formcreator-2.13.0-alpha.3-dev.zip

[amatteo78]

@amatteo78 please test this build. It is based from #2622

glpi-formcreator-2.13.0-alpha.3-dev.zip

@btry Very good, work fine, thanks very much

[btry]

Hi

The branch has been merged. I think I'll release a 2.13.0-alpha.4 version, and it will be probably the last alpha. Next one will be beta or final release.

Thank you for your tests.

Note :

By default no forms are displayed in formlist.php, you need to explicitly select category

There was a misunderstanding about the development of this change. A PR is pending, implementing a new option to show forms immediately, as it was before. The option will let the admin to choose if Formcreator shows all forms or only the forms flagged as default. See #2644