Bug 8222 - properly escape "<" in reviews
properly escape "<" in reviews
Status: RESOLVED INVALID
Product: OJS
Classification: Unclassified
Component: General
2.4.3
All All
: P3 major
Assigned To: PKP Support
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-05-06 19:07 PDT by Nicolas Limare
Modified: 2013-09-06 00:37 PDT (History)
1 user (show)

See Also:
Version Reported In: 2.3.7
Also Affects:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nicolas Limare 2013-05-06 19:07:05 PDT
An editor reported that review reports containing the "<" sign are not properly escaped. The report is cut at this "<" instead of escaping it. Using the "<" symbol is important in math journals, and should not corrupt the report.
Comment 1 Alec Smecher 2013-05-06 19:37:20 PDT
Nicolas, what version of OJS are you using?
Comment 2 Nicolas Limare 2013-05-06 21:17:47 PDT
Alec,

Sorry, the version info in this bug report was wrong. My OJS version is 2.3.7.0, according to the HTML meta headers (is there a better place to look for the version info).

I guess I should first upgrade to the latest version, 2.4.2?

By the way, I expected to be notified of new versions since I registered ny OJS site on http://pkp.sfu.ca/node/add/pkp-site-submission, but so far I only received a bugfix notice, and I supposed there was no new version.
Comment 3 Alec Smecher 2013-05-07 10:42:09 PDT
Nicolas, can you attach a (sanitized as needed) copy of the failed report to this bug entry? Also, a few quick questions:

- What are you importing the CSV file into? (Excel, OpenOffice, ...)
- When entering review comments, do reviewers see the rich text editor (with Bold, Italic, Underline, and other rich text controls)?
Comment 4 Nicolas Limare 2013-05-08 03:42:46 PDT
> Nicolas, can you attach a (sanitized as needed) copy of the failed report to
> this bug entry?

I investigated in details. In fact, the review is truncated when imported into the mail with the editor decision. The review message had this passage:

8<----------8<----------8<----------8<----------8<----------8<----------
- the algorithm.

In the algorithm description, there are strange symbols instead of
"<". Furthermore, I think I prefer the assignment symbol (left
arrow) instead of "=" in algorithms.
8<----------8<----------8<----------8<----------8<----------8<----------

In the mail, it was cut at the "<".

> - What are you importing the CSV file into? (Excel, OpenOffice, ...)

I am not manipulating any CSV file, and I don't understand why this question appears here. ??? 

> - When entering review comments, do reviewers see the rich text editor (with
> Bold, Italic, Underline, and other rich text controls)?

No, TinyMCE is disabled.
Comment 5 Alec Smecher 2013-05-08 08:35:29 PDT
Hi Nicolas -- you referred to "review reports" which I took to mean Journal Management > Stats & Reports > Review Report. It sounds like you're referring instead to the email from the Editor to the Reviewer into which peer reviews can be imported and modified for the author's sake. Is that correct?
Comment 6 Alec Smecher 2013-05-08 08:38:13 PDT
...sorry, I meant "from the Editor to the Author".
Comment 7 Nicolas Limare 2013-05-08 19:52:09 PDT
> Hi Nicolas -- you referred to "review reports" which I took to mean Journal
> Management > Stats & Reports > Review Report. It sounds like you're
> referring instead to the email from the Editor to the Reviewer into which
> peer reviews can be imported and modified for the author's sake. Is that
> correct?


Yes. "review reports" was a (bad) shortcut for "report written by the reviewers and transmitted to the authors by the editor". Sorry for the confusion.
Comment 8 Alec Smecher 2013-05-09 09:31:54 PDT
OK; the problem is TinyMCE being disabled. With TinyMCE disabled, the reviewer comment boxes expect HTML to be entered directly, so reviewers will need to enter &lt; instead of < and &gt; instead of > or else these characters will be interpreted as part of malformed HTML tags. Is there a particular reason you've disabled the TinyMCE plugin?
Comment 9 Nicolas Limare 2013-09-05 00:04:05 PDT
(In reply to comment #8)
> OK; the problem is TinyMCE being disabled. With TinyMCE disabled, the
> reviewer comment boxes expect HTML to be entered directly, so reviewers will
> need to enter &lt; instead of < and &gt; instead of > or else these
> characters will be interpreted as part of malformed HTML tags. Is there a
> particular reason you've disabled the TinyMCE plugin?

Sorry for the (very) late answer. I disabled TinyMCE because I wanted to provide an interface as simple as possible, and our reviewers are used to non-wysiwyg interfaces (LaTeX etc, not Word). Moreover, these comments are expected to be transmitted by mail, and plain text (non-html) mail is preferred here. So the visual enhancements provided by TinyMCE seem not necessary.

I expected that the comment boxes without TinyMCE would take plain text. Expecting people to write HTML by hand seems unrealistic to me, and if it is by design then I think disabling TinyMCE should not be allowed. If you really want some html, why not just wrap the plain text in <pre>...</pre>?

I agree with tagging this bug as invalid. I'd like to see an improvement in the wishlist.
Comment 10 Alec Smecher 2013-09-05 09:57:06 PDT
Nicolas, we're actually moving towards using HTML in emails (with of course a text fallback), but for the moment you're correct, HTML entered via TinyMCE is reduced to plain text before sending. I can suggest two solutions:

1) Adjust the TinyMCE plugin to simplify the interface; see plugins/generic/tinymce/TinyMCEPlugin.inc.php

2) Remove TinyMCE support specifically from the comment boxes; see again plugins/generic/tinymce/TinyMCEPlugin.inc.php, but also several templates where comment contents are passed through "strip_unsafe_html" (you'll want to use "escape" instead) and classes/submission/sectionEditor/SectionEditorHandler.inc.php where they're formatted for emailing. This will basically be a reversal of bug #3683.
Comment 11 Nicolas Limare 2013-09-06 00:37:51 PDT
Alec,

Thanks for your suggestions.

I tried to remove some TinyMCE formatting options, let's see how it works.