Ensure identifiers are properly displayed in the plan viewer. Fixes #3821

docs/en_US/release_notes_3_7.rst

@@ -27,4 +27,5 @@ Bug fixes
 | `Bug #3757 <https://redmine.postgresql.org/issues/3757>`_ - Hide Radio buttons that should not be shown on the maintenance dialogue.
 | `Bug #3797 <https://redmine.postgresql.org/issues/3797>`_ - Prevent attempts to bulk-drop schema objects.
 | `Bug #3798 <https://redmine.postgresql.org/issues/3798>`_ - Ensure the browser toolbar buttons work in languages other than English.
-| `Bug #3805 <https://redmine.postgresql.org/issues/3805>`_ - Allow horizontal sizing of the edit grid text pop-out.
+| `Bug #3805 <https://redmine.postgresql.org/issues/3805>`_ - Allow horizontal sizing of the edit grid text pop-out.
+| `Bug #3821 <https://redmine.postgresql.org/issues/3821>`_ - Ensure identifiers are properly displayed in the plan viewer.

web/pgadmin/feature_tests/xss_checks_panels_and_query_tool_test.py

@@ -9,6 +9,8 @@
 
 from regression.python_test_utils import test_utils
 from regression.feature_utils.base_feature_test import BaseFeatureTest
+from selenium.webdriver import ActionChains
+import sys
 
 
 class CheckForXssFeatureTest(BaseFeatureTest):
@@ -55,6 +57,10 @@ class CheckForXssFeatureTest(BaseFeatureTest):
         self._check_xss_in_query_tool()
         self.page.close_query_tool()
 
+        # Explain module
+        self._check_xss_in_explain_module()
+        self.page.close_query_tool()
+
     def after(self):
         self.page.remove_server(self.server)
 
@@ -68,6 +74,10 @@ class CheckForXssFeatureTest(BaseFeatureTest):
         self.page.select_tree_item(self.test_table_name)
 
     def _check_xss_in_browser_tree(self):
+        print(
+            "\n\tChecking the Browser tree for the XSS",
+            file=sys.stderr, end=""
+        )
         # Fetch the inner html & check for escaped characters
         source_code = self.page.find_by_xpath(
             "//*[@id='tree']"
@@ -80,6 +90,10 @@ class CheckForXssFeatureTest(BaseFeatureTest):
         )
 
     def _check_xss_in_properties_tab(self):
+        print(
+            "\n\tChecking the Properties tab for the XSS",
+            file=sys.stderr, end=""
+        )
         self.page.click_tab("Properties")
         source_code = self.page.find_by_xpath(
             "//span[contains(@class,'uneditable-input')]"
@@ -91,6 +105,10 @@ class CheckForXssFeatureTest(BaseFeatureTest):
         )
 
     def _check_xss_in_sql_tab(self):
+        print(
+            "\n\tChecking the SQL tab for the XSS",
+            file=sys.stderr, end=""
+        )
         self.page.click_tab("SQL")
         # Fetch the inner html & check for escaped characters
         source_code = self.page.find_by_xpath(
@@ -106,6 +124,10 @@ class CheckForXssFeatureTest(BaseFeatureTest):
 
     # Create any constraint with xss name to test this
     def _check_xss_in_dependents_tab(self):
+        print(
+            "\n\tChecking the Dependents tab for the XSS",
+            file=sys.stderr, end=""
+        )
         self.page.click_tab("Dependents")
 
         source_code = self.page.find_by_xpath(
@@ -119,10 +141,17 @@ class CheckForXssFeatureTest(BaseFeatureTest):
             "Dependents tab (BackGrid)"
         )
 
-    def _check_xss_in_query_tool(self):
+    def _open_query_tool(self):
         self.page.driver.find_element_by_link_text("Tools").click()
         self.page.find_by_partial_link_text("Query Tool").click()
         self.page.click_tab('Query -')
+
+    def _check_xss_in_query_tool(self):
+        print(
+            "\n\tChecking the SlickGrid cell for the XSS",
+            file=sys.stderr, end=""
+        )
+        self._open_query_tool()
         self.page.fill_codemirror_area_with(
             "select '<img src=\"x\" onerror=\"console.log(1)\">'"
         )
@@ -144,6 +173,52 @@ class CheckForXssFeatureTest(BaseFeatureTest):
             "Query tool (SlickGrid)"
         )
 
+    def _check_xss_in_explain_module(self):
+        print(
+            "\n\tChecking the Graphical Explain plan for the XSS ...",
+            file=sys.stderr, end=""
+        )
+        self._open_query_tool()
+        self.page.fill_codemirror_area_with(
+            'select * from "{0}"'.format(self.test_table_name)
+        )
+
+        query_op = self.page.find_by_id("btn-query-dropdown")
+        query_op.click()
+
+        self.page.find_by_id("btn-explain").click()
+        self.page.wait_for_query_tool_loading_indicator_to_disappear()
+        self.page.click_tab('Explain')
+
+        for idx in range(3):
+            # Re-try logic
+            try:
+                ActionChains(self.driver).move_to_element(
+                    self.driver.find_element_by_css_selector(
+                        'div.pgadmin-explain-container > svg > g > g > image'
+                    )
+                ).perform()
+                break
+            except Exception as e:
+                if idx != 2:
+                    continue
+                else:
+                    print(
+                        "\n\tUnable to locate the explain container to check"
+                        " the image tooltip for XSS",
+                        file=sys.stderr, end=""
+                    )
+                    raise
+
+        source_code = self.driver.find_element_by_id(
+            'toolTip').get_attribute('innerHTML')
+
+        self._check_escaped_characters(
+            source_code,
+            "&lt;h1&gt;X",
+            "Explain tab (Graphical explain plan)"
+        )
+
     def _check_escaped_characters(self, source_code, string_to_find, source):
         # For XSS we need to search against element's html code
         assert source_code.find(string_to_find) != - \

web/pgadmin/misc/static/explain/js/explain.js

@@ -673,12 +673,12 @@ define('pgadmin.misc.explain', [
           key !== 'image_text' && key !== 'xpos' &&
           key !== 'ypos' && key !== 'width' &&
           key !== 'height') {
-          title += key + ': ' + value + '\n';
+          title += `${key}: ${value}\n`;
         }
       });
 
       title += '</title>';
-      // this.title = Snap.parse(title);
+
       image.append(Snap.parse(title));
 
       image.mouseover(() => {
@@ -697,7 +697,14 @@ define('pgadmin.misc.explain', [
             key !== 'image_text' && key !== 'xpos' &&
             key !== 'ypos' && key !== 'width' &&
             key !== 'height') {
-            tooltip.append('<tr><td class="label explain-tooltip">' + key + '</td><td class="label explain-tooltip-val">' + value + '</td></tr>');
+            key = _.escape(key);
+            value = _.escape(value);
+            tooltip.append(`
+              <tr>
+                <td class="label explain-tooltip">${key}</td>
+                <td class="label explain-tooltip-val">${value}</td>
+              </tr>
+            `);
           }
         });
 

web/pgadmin/misc/static/explain/js/explain_statistics.js

@@ -18,7 +18,6 @@ let StatisticsModel = Backbone.Model.extend({
     }
 
     $('.pg-explain-stats-area').on('mouseover', () => {
-
       // Empty the tooltip content if it has any and add new data
       toolTipContainer.empty();
       if (Object.keys(jit_stats).length == 0 &&
@@ -33,9 +32,14 @@ let StatisticsModel = Backbone.Model.extend({
       if (Object.keys(jit_stats).length > 0){
         tooltip.append('<tr><td class="label explain-tooltip">JIT:</td></tr>');
         _.each(jit_stats, function(value, key) {
-          tooltip.append('<tr><td class="label explain-tooltip">&nbsp&nbsp'
-          + key + '</td><td class="label explain-tooltip-val">'
-          + value + '</td></tr>');
+          key = _.escape(key);
+          value = _.escape(value);
+          tooltip.append(`
+            <tr>
+              <td class="label explain-tooltip">  ${key}</td>
+              <td class="label explain-tooltip-val">${value}</td>
+            </tr>
+          `);
         });
       }
 
@@ -45,20 +49,35 @@ let StatisticsModel = Backbone.Model.extend({
           if (triggers instanceof Object) {
             _.each(triggers, function(value, key) {
               if (key === 'Trigger Name') {
-                tooltip.append('<tr><td class="label explain-tooltip">&nbsp;&nbsp;'
-                + key + '</td><td class="label explain-tooltip-val">'
-                + value + '</td></tr>');
+                key = _.escape(key);
+                value = _.escape(value);
+                tooltip.append(`
+                  <tr>
+                    <td class="label explain-tooltip">  ${key}</td>
+                    <td class="label explain-tooltip-val">${value}</td>
+                  </tr>
+                `);
               } else {
-                tooltip.append('<tr><td class="label explain-tooltip">&nbsp;&nbsp;&nbsp;&nbsp;'
-                + key + '</td><td class="label explain-tooltip-val">'
-                + value + '</td></tr>');
+                key = _.escape(key);
+                value = _.escape(value);
+                tooltip.append(`
+                  <tr>
+                    <td class="label explain-tooltip">    ${key}</td>
+                    <td class="label explain-tooltip-val">${value}</td>
+                  </tr>
+                `);
               }
             });
           }
           else {
-            tooltip.append('<tr><td class="label explain-tooltip">&nbsp;&nbsp;'
-            + key_id + '</td><td class="label explain-tooltip-val">'
-            + triggers + '</td></tr>');
+            key_id = _.escape(key_id);
+            triggers = _.escape(triggers);
+            tooltip.append(`
+              <tr>
+                <td class="label explain-tooltip">  ${key_id}</td>
+                <td class="label explain-tooltip-val">${triggers}</td>
+              </tr>
+            `);
           }
         });
       }