Rework the error system

There are several problems with the current error system of FusionDirectory:

1. The error is passed as a text string with no context (class, line, object, attribute, …)
2. The error text may contain HTML, and because of this is not always escaped as it should (see #5907)

The idea is to replace error strings with objects which contain:

1. As much context as possible (class, file, line, object dn, object type, attribute concerned if any, … Which context is stored will depend of error types)
2. A ready to print error text with HTML tags
3. An text-only version of the error, for webservice purposes (should be computed from the other one)

This would allow webservice to have detailed information on an error, especially for check errors, to know which attribute triggers the error. It would avoid the webservice receiving HTML in the error message. It would avoid escaping problems in the UI. When rendering the error in FD, it would allow to give some context if available, in a standardized manner. If debug is active, it will allow to give a trace of the actual error and not of the call to msg_dialog::display like the current situation.

This class should have a __toString method to fallback to string rendering for backward compatibility. This should allow us to have both error objects and error strings for the transition.

Linked ticket

Error dialogs do not escape HTML from fields https://gitlab.fusiondirectory.org/fusiondirectory/fd/-/issues/5907

changed milestone to %FusionDirectory 1.5

By Côme Chilliet on 2020-02-13T10:26:55 (imported from GitLab)

added fusiondirectory-core label

added 30m of time spent at 2020-02-13

By Côme Chilliet on 2020-02-13T10:30:48 (imported from GitLab)

changed milestone to %FusionDirectory 1.4

By bmortier on 2020-03-21T14:27:13 (imported from GitLab)

added PJ1802-0188 label

added error management label

changed the description

By bmortier on 2020-05-15T14:46:19 (imported from GitLab)

changed the description

By bmortier on 2020-05-15T14:46:44 (imported from GitLab)

added telecom sud paris label

@MCMic,

this work should be done on a branch not merged and that we can build apart to see all the issues

Cheers

By bmortier on 2020-06-03T07:16:05 (imported from GitLab)

created merge request !781 to address this issue

By Côme Chilliet on 2020-06-03T10:17:17 (imported from GitLab)

mentioned in merge request !781

By Côme Chilliet on 2020-06-03T10:17:17 (imported from GitLab)

mentioned in commit 114cc50e

By Côme Chilliet on 2020-06-03T15:04:20 (imported from GitLab)

We need to refactor HTML escaping in relation with translations.

In the current situation, we may or may not escape the translated string depending on the situation, and the translators have no way to know if they need to output HTML or text, except in obvious cases (like when the origin string contains HTML already).

We need to either find a way to make it clear if a string is HTML or text (but I think our gettext setup does not support context string?), or to take a decision that translated string are always HTML or always text. I would favor always text, as it will ease translator work, and avoid vulnerabilities coming from translation.

By Côme Chilliet on 2020-06-04T09:38:45 (imported from GitLab)

We need to either find a way to make it clear if a string is HTML or text (but I think our gettext setup does not support context string?), or to take a decision that translated string are always HTML or always text. I would favor always text, as it will ease translator work, and avoid vulnerabilities coming from translation.

@MCMic i would vote for text is there some situations where we need html in translations ?

By bmortier on 2020-06-04T09:38:45 (imported from GitLab)

We’ll need to sort out the following cases:

$   grep "<" /usr/share/fusiondirectory/locale/en/fusiondirectory.po
# FIRST AUTHOR <EMAIL@ADDRESS>, YEAR.
"Last-Translator: FusionDirectory project <contact@fusiondirectory.org>\n"
"It seems you are trying to decode something which is not encoded : %s<br/>\n"
msgid "Warning: <a href=\"%s\">Session is not encrypted!</a>"
msgid "You have no permission to modify the object:<br/>%s"
msgid "You have no permission to modify these objects:<br/>%s"
"It seems your user password has expired. Please use <a href=\"recovery.php"
"\">password recovery</a> to change it."
msgid "Entry will be moved from:<br/>\t%s<br/>to:<br/>\t%s"
"PHP setup configuration (<a href=\"?info\" target=\"_blank\">show "
"information</a>)"
"This dialog provides a simple way to change your password.<br/> Enter the "
"&copy; 2002-%1 <a href=\"http://www.fusiondirectory.org\">The "
"FusionDirectory team, %2</a>"
msgid "<strong>%1</strong> references our <strong>%3</strong>"
"<strong>%1</strong> references our field <strong>%3</strong> from tab "
"<strong>%2</strong>"

For <br/> we can use nl2br after escaping, for <strong> and <a> we’ll need to be smarter. I’ll make an attempt at fixing this.

By Côme Chilliet on 2020-06-04T10:21:00 (imported from GitLab)

mentioned in commit da53befc

By Côme Chilliet on 2020-06-04T10:18:21 (imported from GitLab)

mentioned in commit 2ba82788

By Côme Chilliet on 2020-06-04T10:18:21 (imported from GitLab)

added 2h 30m of time spent at 2020-06-04

By Côme Chilliet on 2020-06-04T10:18:50 (imported from GitLab)

mentioned in commit 8527c44c

By Côme Chilliet on 2020-06-09T12:43:12 (imported from GitLab)

mentioned in commit f77e2fcd

By Côme Chilliet on 2020-06-09T12:43:12 (imported from GitLab)

mentioned in commit 1c765c3a

By Côme Chilliet on 2020-06-09T13:58:07 (imported from GitLab)

mentioned in commit 88d27cae

By Côme Chilliet on 2020-06-09T13:58:07 (imported from GitLab)

mentioned in commit 687055ee

By Côme Chilliet on 2020-06-09T15:01:22 (imported from GitLab)

mentioned in commit 817f6e7d

By Côme Chilliet on 2020-06-09T15:01:22 (imported from GitLab)

added 6h of time spent at 2020-06-09

By Côme Chilliet on 2020-06-09T15:19:30 (imported from GitLab)

mentioned in commit 4ad5b8b9

By Côme Chilliet on 2020-06-09T15:23:06 (imported from GitLab)

mentioned in commit 78e80de6

By Côme Chilliet on 2020-06-09T15:24:19 (imported from GitLab)

@bmortier Now it would make sense to build the branch to be able to test the changes.

By Côme Chilliet on 2020-06-09T15:27:12 (imported from GitLab)

mentioned in commit ca5305ed

By Côme Chilliet on 2020-06-10T10:44:51 (imported from GitLab)

mentioned in commit b82dc1aa

By Côme Chilliet on 2020-06-10T10:44:51 (imported from GitLab)

mentioned in commit f40c5ea8

By Côme Chilliet on 2020-06-10T10:44:52 (imported from GitLab)

mentioned in commit 7ea74130

By Côme Chilliet on 2020-06-10T14:02:58 (imported from GitLab)

mentioned in commit 58b6d8d7

By Côme Chilliet on 2020-06-10T15:13:52 (imported from GitLab)

added 6h of time spent at 2020-06-10

By Côme Chilliet on 2020-06-10T15:15:30 (imported from GitLab)

mentioned in commit 31437959

By Côme Chilliet on 2020-06-11T09:07:27 (imported from GitLab)

mentioned in commit 27c8c229

By Côme Chilliet on 2020-06-11T09:07:27 (imported from GitLab)

mentioned in commit b997e2ca

By Côme Chilliet on 2020-06-11T09:07:27 (imported from GitLab)

mentioned in commit 61faa525

By Côme Chilliet on 2020-06-11T09:58:47 (imported from GitLab)

mentioned in commit 902c60ca

By Côme Chilliet on 2020-06-11T09:58:47 (imported from GitLab)

mentioned in commit 9057eda6

By Côme Chilliet on 2020-06-11T09:58:47 (imported from GitLab)

added 3h of time spent at 2020-06-11

By Côme Chilliet on 2020-06-11T09:59:23 (imported from GitLab)

mentioned in issue fd-plugins#6061 (closed)

By Côme Chilliet on 2020-06-16T07:48:12 (imported from GitLab)

mentioned in commit de72ac82

By Côme Chilliet on 2020-06-16T12:15:14 (imported from GitLab)

mentioned in commit 9831cd19

By Côme Chilliet on 2020-06-16T12:15:14 (imported from GitLab)

added 6h of time spent at 2020-06-16

By Côme Chilliet on 2020-06-16T15:01:36 (imported from GitLab)

Error message before the changes:

Error message with current MR:

I guess the information on where the error comes from (first line with the ">") should be shown with another look, but not sure how, maybe just a smaller font. Also the description of the attribute is not shown currently but I would like to add it as well, not sure where and with what look.

Other differences are that the trace is way more precise than before (it starts in IntAttribute, not management), and through the webservice we get error context and no HTML. Also if the field has an empty label the ldap name is shown as italic instead.

By Côme Chilliet on 2020-06-17T12:46:00 (imported from GitLab)

@MCMic very nice work

maybe

dn: uid=fd-admin,ou=people,dc=xx,dc=xx
Type : Utilisateur attribute : Code postal

By bmortier on 2020-06-17T12:46:00 (imported from GitLab)

"Utilisateur" is not the type but the tab. I do not show the type because in a lot of cases it’s the same word as the tab.

I think it will be too much space in we split everything in lines. I’m gonna try showing it with a smaller font, with the description of the attribute as well.

By Côme Chilliet on 2020-06-17T13:38:40 (imported from GitLab)

@MCMic,

then

dn: uid=fd-admin,ou=people,dc=xx,dc=xx  
tab: utilisateur, attribute: Code Postal  

its only one line more because the uid part is already on one line

we should not reduce the font because we already got remark that some time the font are to small.

By bmortier on 2020-06-17T13:42:08 (imported from GitLab)

added 2h of time spent at 2020-06-17

By Côme Chilliet on 2020-06-17T10:04:00 (imported from GitLab)

mentioned in commit 66e546d5

By Côme Chilliet on 2020-06-17T10:05:23 (imported from GitLab)

added 5m of time spent at 2020-06-17

By bmortier on 2020-06-17T12:46:00 (imported from GitLab)

added 3h of time spent at 2020-06-17

By Côme Chilliet on 2020-06-17T15:29:14 (imported from GitLab)

mentioned in commit aed8c3b6

By Côme Chilliet on 2020-06-18T08:05:56 (imported from GitLab)