@laemtl were you able to identify where it was being encoded in the first place ? the PR @johnsaigle linked in the issue (#6223) was the result of sequentially encoding the data during the process of saving.

Is it the same case here ? is the data being encoded several times before being saved and if so, can you identify where it is happening ?

@ridz1208 Yes, it is the same case here, encoding happens on save, so the encoded version is saved in the db. My understanding of the problem is the data is not encoded twice, but the string is not decoded on load so the encoded characters are interpreted as plain text.

@laemtl yeah I just did my own little investigation. your are correct.

It is slightly different then the instrument bug because in this case its a single escape happening but when the data is loaded, it's being loaded into a static field and the browser does not seem to un-escape static fields. so the bug here is not to remove an escaping step but rather to add an unescaping step

which brings the question @johnsaigle whats the use of escaping the data if it's gonna get unescaped before being sent to the browser...

whats the use of escaping the data if it's gonna get unescaped before being sent to the browser...

The best way to handle data safely is to:

• to validation on data as it comes in (reject bad data)
• "sanitize" the output as far downstream in the rendering as possible (i.e. Smarty/React)

We have a bunch of escaping/unescaping because:

• LORIS (still) passes around a bunch of raw HTML. (Mostly because of instruments but also some core stuff). This causes Smarty to break if we enable its autoescaping feature (Set smarty's autoescape template variable option #2956)
• Modules are very ad-hoc and it makes it difficult to determine in one central place where output can be sanitized
• Not everything is React. React safely handles data in innerHTML by default which takes care of a lot of our security needs. (Props are not sanitized, however.)
• Old codebase, not enough development time, refactoring needs to be prioritized, code rot, etc etc etc

I'll make a detailed issue about this because this is a problem that we should track and it would be best to have a place I can refer people to to explain why the codebase behaves the way it does.

@ridz1208 See #6648

@johnsaigle what would be your recommendation for this release ? just unescape the HTML for now ?

@ridz1208 Yeah this is a very deep and longstanding problem. We won't be able to solve it for this release. Until we have the time and will to fix the root of this problem, we will have to keep unescaping things as bugs arise.