From de3a37d7fb266f2e9e6719eb966fe5a5d8e6c9bb Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinkle@fastmail.com>
Date: Sun, 7 Jul 2024 02:02:41 +0100
Subject: [PATCH] HTML Reporter: Fix encoding of label for urlConfig
 multi-value item

* Add test coverage for QUnit.config.urlConfig items with object
  as value. The urlConfig feature is mostly covered by
  `/test/reporter-urlparams.html` already, but what isn't covered yet
  is an item with an object value, where option labels can be
  customised.

* In adding tests I noticed that, when urlConfig is used to create
  a multi-value option (rather than string, as is more common), then
  `val.label` was not escaped, which meant that if labels were to
  contain mention of an HTML tag or otherwise contain "<" and ">",
  these could glitch and break part of the toolbar rendering.
  This is unlikely to be exploitable, e.g. not controlled by
  URL parameters, and generally populated with literals. Even dynamic
  menus that feed dropdown contents from external input are fine,
  since this affects the top-level label only.

Improve docs:
* Avoid the overloaded word "option". Instead using the word "item"
  consistently when referring to urlConfig items,
  to avoid confusion with the `<select>` options inside of the array.
* Remove odd inline snippet using `value = ;`. This was solely to
  satisfy ESLint run on Markdown, but is simply confusing. The array
  was redundant with the full example lower down, and I've now added
  a full example with an object value as well.
---
 docs/api/config/urlConfig.md  | 38 ++++++++--------
 src/reporters/HtmlReporter.js |  2 +-
 test/main/HtmlReporter.js     | 85 +++++++++++++++++++++++++++++++++--
 3 files changed, 102 insertions(+), 23 deletions(-)

diff --git a/docs/api/config/urlConfig.md b/docs/api/config/urlConfig.md
index ddd64d920..9c0ba7be2 100644
--- a/docs/api/config/urlConfig.md
+++ b/docs/api/config/urlConfig.md
@@ -28,7 +28,7 @@ This property controls which form controls display in the QUnit toolbar. By defa
 Each array item should be an object shaped as follows:
 
 ```js
-({
+QUnit.config.urlConfig.push({
   id: string,
   label: string,
   tooltip: string, // optional
@@ -42,24 +42,13 @@ Each array item should be an object shaped as follows:
 
 Each item may also have a `value` property:
 
-If `value` is undefined, the option will render as a checkbox. The corresponding URL parameter will be set to "true" when the checkbox is checked, and otherwise will be absent.
-
-If `value` is a string, the option will render as a checkbox. The corresponding URL parameter will be set to the value when the checkbox is checked, and otherwise will be absent.
+If `value` is undefined, the item will render as a checkbox. The corresponding URL parameter will be set to "true" when the checkbox is checked, and otherwise will be absent.
 
-If `value` is an array, the option will render as a "select one" menu with an empty value as first default option, followed by one option for each item in the array. The corresponding URL parameter will be absent when the empty option is selected, and otherwise will be set to the value of the selected array item.
+If `value` is a string, the item will render as a checkbox. The corresponding URL parameter will be set to the value when the checkbox is checked, and otherwise will be absent.
 
-```js
-value = [ 'foobar', 'baz' ];
-```
+If `value` is an array, the item will render as a "select one" dropdown menu with an empty value as first default option, followed by one option for each item in the array. The corresponding URL parameter will be absent when the empty option is selected, and otherwise will be set to the value of the selected array item.
 
-If `value` is an object, the option will render as a "select one" menu as for an array. The keys will be used as option values, and the values will be used as option display labels. The corresponding URL parameter will be absent when the empty option is selected, and otherwise will be set to the object key of the selected property.
-
-```js
-value = {
-  foobar: 'Foo with bar',
-  baz: 'Baz'
-};
-```
+If `value` is an object, the item will render as a dropdown menu. The URL parameter will be set to the key of the selected property, and this will also be available via `QUnit.urlParams[id]`. The object values will be used as display label for each option in the dropdown menu. The corresponding URL parameter will be absent when the empty option is selected.
 
 ## Examples
 
@@ -82,8 +71,21 @@ Add a dropdown to the toolbar.
 ```js
 QUnit.config.urlConfig.push({
   id: 'jquery',
-  label: 'jQuery version',
+  label: 'jQuery',
   value: [ '1.7.2', '1.8.3', '1.9.1' ],
-  tooltip: 'Which jQuery version to test against.'
+  tooltip: 'Which version of jQuery to test against.'
 });
 ```
+
+```js
+QUnit.config.urlConfig.push({
+  id: 'jquery',
+  label: 'jQuery',
+  value: { '1.7.2': 'v1.7.2 (LTS)', '1.8.3': 'v1.8.3 (Current)' },
+  tooltip: 'Which version of jQuery to test against.'
+});
+
+if (QUnit.urlConfig.jquery) {
+  // Load jQuery
+}
+```
diff --git a/src/reporters/HtmlReporter.js b/src/reporters/HtmlReporter.js
index 05cac7b91..ccf1e09e7 100644
--- a/src/reporters/HtmlReporter.js
+++ b/src/reporters/HtmlReporter.js
@@ -79,7 +79,7 @@ function getUrlConfigHtml (config) {
     } else {
       let selection = false;
       urlConfigHtml += "<label for='qunit-urlconfig-" + escaped +
-        "' title='" + escapedTooltip + "'>" + val.label +
+        "' title='" + escapedTooltip + "'>" + escapeText(val.label) +
         ": <select id='qunit-urlconfig-" + escaped +
         "' name='" + escaped + "' title='" + escapedTooltip + "'><option></option>";
 
diff --git a/test/main/HtmlReporter.js b/test/main/HtmlReporter.js
index 6291a6d14..23daf76cb 100644
--- a/test/main/HtmlReporter.js
+++ b/test/main/HtmlReporter.js
@@ -3,6 +3,18 @@
 // Skip in non-browser environment
 (typeof document !== 'undefined' ? QUnit.module : QUnit.module.skip)('HtmlReporter', {
   beforeEach: function () {
+    this.normHTML = function (html) {
+      var div = document.createElement('div');
+      // For example:
+      // * Firefox 45 (Win8.1) normalizes attribute order differently from Firefox 127 (macOS).
+      // * IE 11 (Win10) will try to minimise escaping by using single quotes for values
+      //   that contain double quotes.
+      // * Safari 9.1 will serialize "<i>" tag as escaped entities in attributes, whereas other
+      //   browsers specifically normalize in the other direction where "<i>" is a literal inside
+      //   quoted attributes where escaping for "<" is redundant.
+      div.innerHTML = html;
+      return div.innerHTML;
+    };
     this.MockQUnit = {
       on: function (event, fn) {
         this.on[event] = fn;
@@ -215,6 +227,47 @@ QUnit.test('hidepassed', function (assert) {
   assert.strictEqual(node.checked, true);
 });
 
+QUnit.test('urlConfig', function (assert) {
+  var element = document.createElement('div');
+  new QUnit.reporters.html(this.MockQUnit, {
+    element: element,
+    config: {
+      urlConfig: [
+        'xid',
+        {
+          id: 'xmulti',
+          label: 'Multi',
+          tooltip: 'Escaped "><i>'
+        },
+        {
+          id: 'xmenu',
+          label: 'Menu',
+          tooltip: 'My tooltip',
+          value: ['a', 'b']
+        },
+        {
+          id: 'xmenulabel',
+          label: 'Menu',
+          value: { a: 'AA', bbb: 'B' }
+        },
+        // Create UI for a built-in option
+        'altertitle'
+      ]
+    }
+  });
+  this.MockQUnit._do_start_empty();
+
+  var node = element.querySelector('.qunit-url-config');
+  assert.strictEqual(node.innerHTML, this.normHTML(
+    '<label for="qunit-urlconfig-xid" title=""><input id="qunit-urlconfig-xid" name="xid" type="checkbox" title="">xid</label>' +
+      '<label for="qunit-urlconfig-xmulti" title="Escaped &quot;><i>"><input id="qunit-urlconfig-xmulti" name="xmulti" type="checkbox" title="Escaped &quot;><i>">Multi</label>' +
+      '<label for="qunit-urlconfig-xmenu" title="My tooltip">Menu: <select id="qunit-urlconfig-xmenu" name="xmenu" title="My tooltip"><option></option><option value="a">a</option><option value="b">b</option></select></label>' +
+      '<label for="qunit-urlconfig-xmenulabel" title="">Menu: <select id="qunit-urlconfig-xmenulabel" name="xmenulabel" title=""><option></option><option value="a">AA</option><option value="bbb">B</option></select></label>' +
+      '<label for="qunit-urlconfig-altertitle" title=""><input id="qunit-urlconfig-altertitle" name="altertitle" type="checkbox" title="">altertitle</label>',
+    'qunit-url-config HTML'
+  ));
+});
+
 QUnit.test('filter', function (assert) {
   var element = document.createElement('div');
   new QUnit.reporters.html(this.MockQUnit, {
@@ -251,7 +304,24 @@ QUnit.test('overall escaping', function (assert) {
   new QUnit.reporters.html(this.MockQUnit, {
     element: element,
     config: {
-      urlConfig: []
+      urlConfig: [
+        {
+          id: 'x',
+          label: '<script id="oops-urlconfig">window.oops="urlconfig-x-label";</script>',
+          tooltip: '<script id="oops-urlconfig">window.oops="urlconfig-x-tip";</script>',
+          value: {
+            '<script id="oops-urlconfig">window.oops="urlconfig-x-key";</script>': '<script id="oops-urlconfig">window.oops="urlconfig-y-label";</script>'
+          }
+        },
+        {
+          id: 'y',
+          label: '<script id="oops-urlconfig">window.oops="urlconfig-y-label";</script>',
+          tooltip: '<script id="oops-urlconfig">window.oops="urlconfig-y-tip";</script>',
+          value: [
+            '<script id="oops-urlconfig">window.oops="urlconfig-y-value";</script>'
+          ]
+        }
+      ]
     }
   });
 
@@ -295,9 +365,14 @@ QUnit.test('overall escaping', function (assert) {
     runtime: 2
   });
 
-  var scriptTagPos = element.innerHTML.indexOf('<script');
-  var scriptTags = scriptTagPos !== -1 ? element.innerHTML.slice(scriptTagPos, scriptTagPos + 20) : '';
-  assert.strictEqual(scriptTags, '', 'escape script tags');
+  var scriptTagsCount = element.querySelectorAll('script').length;
+  assert.strictEqual(scriptTagsCount, 0, 'script elements');
+  // This regex is imperfect as it will incorrectly match attribute values where
+  // "<" doesn't need to be escaped. While we do escape it there for simplicity,
+  // accessing innerHTML returns the browser's normalized HTML serialization.
+  // As such, only run this to aid debugging if the above selector finds actual tags.
+  var scriptTagMatch = scriptTagsCount ? element.innerHTML.match(/.{0,100}<script{0,100}/) : null;
+  assert.deepEqual(scriptTagMatch, null, 'escape script tags');
 
   assert.strictEqual(window.oops, undefined, 'prevent eval');
 
@@ -314,9 +389,11 @@ QUnit.test('overall escaping', function (assert) {
     'formatting of test output'
   );
 
+  var oopsUrlConfig = element.querySelector('#oops-urlconfig') || document.querySelector('#oops-urlconfig');
   var oopsModule = element.querySelector('#oops-module') || document.querySelector('#oops-module');
   var oopsTest = element.querySelector('#oops-test') || document.querySelector('#oops-test');
   var oopsAssertion = element.querySelector('#oops-assertion') || document.querySelector('#oops-assertion');
+  assert.strictEqual(oopsUrlConfig, null, 'escape url config');
   assert.strictEqual(oopsModule, null, 'escape module name');
   assert.strictEqual(oopsTest, null, 'escape test name');
   assert.strictEqual(oopsAssertion, null, 'escape assertion message');