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:
    • Testing Instructions:
      Hide
      1. Log in as admin and create a course
      2. Go to the question bank in the course (course page > click the cog icon >  more ... > question bank > questions)
      3. create a true/false new question
      4. Right click on the tag icon that is next to the newly created question and open the link in a new tab
      5. Verify that you see the editing form for the question 
      Show
      Log in as admin and create a course Go to the question bank in the course (course page > click the cog icon >  more ... > question bank > questions) create a true/false new question Right click on the tag icon that is next to the newly created question and open the link in a new tab Verify that you see the editing form for the question 
    • Affected Branches:
      MOODLE_35_STABLE, MOODLE_36_STABLE, MOODLE_37_STABLE
    • Fixed Branches:
      MOODLE_35_STABLE, MOODLE_36_STABLE
    • Pull from Repository:
    • Pull 3.5 Branch:
    • Pull Master Branch:
      MDL-64551-master

      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

            Assignee:
            rezaie9 Shamim Rezaie
            Reporter:
            nicoroeser Nico Roeser
            Peer reviewer:
            Tim Hunt
            Integrator:
            Eloy Lafuente (stronk7)
            Tester:
            Janelle Barcega
            Participants:
            Component watchers:
            Tim Hunt, Andrew Nicols, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze
            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