Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-64551

Double HTML-escaping of ampersand ("&") in tag management links in question bank

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.5.3, 3.6.1, 3.7
    • Fix Version/s: 3.5.5, 3.6.3
    • Component/s: Questions
    • Labels:

      Description

      The question bank of a course (/question/edit.php?courseid=<course_ID>) contains a tag icon whith tooltip "Manage Tags" for each question entry. Clicking on this icon opens a popup for tag management. This is implemented with JavaScript.

      For cases where JavaScript is not used (users who have it disabled, screenreaders, bots, and so on), a link to the question settings is set. Its href attribute is:

      /question/question.php?returnurl=%2Fquestion%2Fedit.php%3Fcourseid%3D<course_ID>&amp;courseid=<course_ID>&amp;id=<question_ID>
      

      This is obviously wrong. (The href attribute contains the link as it should be, but with all ampersand characters ("&") replaced with "& a m p ;" (without the spaces).)

      I have verified that this is a bug in the Moodle 3.5, 3.6 and 3.7 branches, and have not checked other versions.

      Source of this bug is that the link around the "Manage Tags" icon is generated in a way that differs from how the other icons are generated: /question/classes/bank/tags_action_column.php uses html_writer::link for link creation (function core_question\bank\tags_action_column->print_tag_icon, called from function display_content), and the HTML writer of course correctly (HTML-)escapes URIs. But it is passed an URI which has already been HTML-escaped (via moodle_url->out). The other icons use core_question\bank\action_column_base->print_icon (in file /question/classes/bank/action_column_base.php), which does not escape{{ }}$url another time, so they work properly. (But they seem not to HTML-escape $title, which is another bug and may be spun off into another ticket.)

      As I am not too experienced in this certain area, and do not see through all related classes (yet), I will not provide a patch, but can offer a suggestion: I think it may be a good idea to modify action_column_base->print_icon to escape its arguments before writing them out, and change all action column classes to always pass unescaped $url until the URI is actually printed out. In addition to this, override print_icon in tags_action_column to do its special job there. This should be a viable solution, I hope. But I do not know whether an action_link (from class core_renderer) may be better for the "Manage tags" link, as I do not have deep insights into when which link types/functions are internally used by Moodle.

        Attachments

          Activity

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                11/Mar/19

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 15 minutes
                15m