Skip to content

Commit

Permalink
HTML Reporter: Fix encoding of label for urlConfig multi-value item
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
@Krinkle
Krinkle committed Jul 7, 2024
1 parent 37f6e4b commit de3a37d
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 23 deletions.
38 changes: 20 additions & 18 deletions docs/api/config/urlConfig.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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
}
```
2 changes: 1 addition & 1 deletion src/reporters/HtmlReporter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>";

Expand Down
85 changes: 81 additions & 4 deletions test/main/HtmlReporter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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, {
Expand Down Expand Up @@ -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>'
]
}
]
}
});

Expand Down Expand Up @@ -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');

Expand All @@ -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');
Expand Down

0 comments on commit de3a37d

Please sign in to comment.