getSelectorFromElement return null on bad selectors (#27912)

diff --git a/js/src/util.js b/js/src/util.js
index 0d434c4c515cbc026aa72c841045a74a97cbf588..c3e23e81747bb2750c4bca9f862a1fa2133e0018 100644 (file)
--- a/js/src/util.js
+++ b/js/src/util.js
@@ -82,7 +82,11 @@ const Util = {
       selector = hrefAttr && hrefAttr !== '#' ? hrefAttr.trim() : ''
     }
 
-    return selector && document.querySelector(selector) ? selector : null
+    try {
+      return document.querySelector(selector) ? selector : null
+    } catch (err) {
+      return null
+    }
   },
 
   getTransitionDurationFromElement(element) {

diff --git a/js/tests/unit/modal.js b/js/tests/unit/modal.js
index 782a86eea09f889967c4de3be0db4c4c4c52e33a..4d7682aaa7dbb85ec6d0b0044f579b86079972fe 100644 (file)
--- a/js/tests/unit/modal.js
+++ b/js/tests/unit/modal.js
@@ -619,40 +619,37 @@ $(function () {
     assert.expect(1)
     var done = assert.async()
 
-    try {
-      var $toggleBtn = $('<button data-toggle="modal" data-target="&lt;div id=&quot;modal-test&quot;&gt;&lt;div class=&quot;contents&quot;&lt;div&lt;div id=&quot;close&quot; data-dismiss=&quot;modal&quot;/&gt;&lt;/div&gt;&lt;/div&gt;"/>')
-        .appendTo('#qunit-fixture')
+    var $toggleBtn = $('<button data-toggle="modal" data-target="&lt;div id=&quot;modal-test&quot;&gt;&lt;div class=&quot;contents&quot;&lt;div&lt;div id=&quot;close&quot; data-dismiss=&quot;modal&quot;/&gt;&lt;/div&gt;&lt;/div&gt;"/>')
+      .appendTo('#qunit-fixture')
 
-      $toggleBtn.trigger('click')
-    } catch (e) {
+    $toggleBtn.trigger('click')
+    setTimeout(function () {
       assert.strictEqual($('#modal-test').length, 0, 'target has not been parsed and added to the document')
       done()
-    }
+    }, 0)
   })
 
   QUnit.test('should not execute js from target', function (assert) {
     assert.expect(0)
     var done = assert.async()
 
-    try {
-      // This toggle button contains XSS payload in its data-target
-      // Note: it uses the onerror handler of an img element to execute the js, because a simple script element does not work here
-      //       a script element works in manual tests though, so here it is likely blocked by the qunit framework
-      var $toggleBtn = $('<button data-toggle="modal" data-target="&lt;div&gt;&lt;image src=&quot;missing.png&quot; onerror=&quot;$(&apos;#qunit-fixture button.control&apos;).trigger(&apos;click&apos;)&quot;&gt;&lt;/div&gt;"/>')
-        .appendTo('#qunit-fixture')
-      // The XSS payload above does not have a closure over this function and cannot access the assert object directly
-      // However, it can send a click event to the following control button, which will then fail the assert
-      $('<button>')
-        .addClass('control')
-        .on('click', function () {
-          assert.notOk(true, 'XSS payload is not executed as js')
-        })
-        .appendTo('#qunit-fixture')
+    // This toggle button contains XSS payload in its data-target
+    // Note: it uses the onerror handler of an img element to execute the js, because a simple script element does not work here
+    //       a script element works in manual tests though, so here it is likely blocked by the qunit framework
+    var $toggleBtn = $('<button data-toggle="modal" data-target="&lt;div&gt;&lt;image src=&quot;missing.png&quot; onerror=&quot;$(&apos;#qunit-fixture button.control&apos;).trigger(&apos;click&apos;)&quot;&gt;&lt;/div&gt;"/>')
+      .appendTo('#qunit-fixture')
+    // The XSS payload above does not have a closure over this function and cannot access the assert object directly
+    // However, it can send a click event to the following control button, which will then fail the assert
+    $('<button>')
+      .addClass('control')
+      .on('click', function () {
+        assert.notOk(true, 'XSS payload is not executed as js')
+      })
+      .appendTo('#qunit-fixture')
 
-      $toggleBtn.trigger('click')
-    } catch (e) {
-      done()
-    }
+    $toggleBtn.trigger('click')
+
+    setTimeout(done, 500)
   })
 
   QUnit.test('should not try to open a modal which is already visible', function (assert) {

diff --git a/js/tests/unit/util.js b/js/tests/unit/util.js
index cb938314330bc1126e4733360a389ada9bb55e81..858fee6f4d024dce816edf905d999f2f55b43d2e 100644 (file)
--- a/js/tests/unit/util.js
+++ b/js/tests/unit/util.js
@@ -20,17 +20,16 @@ $(function () {
     assert.strictEqual(Util.getSelectorFromElement($el2[0]), null)
   })
 
-  QUnit.test('Util.getSelectorFromElement should throw error when there is a bad selector', function (assert) {
+  QUnit.test('Util.getSelectorFromElement should return null when there is a bad selector', function (assert) {
     assert.expect(2)
 
     var $el = $('<div data-target="#1"></div>').appendTo($('#qunit-fixture'))
 
-    try {
-      assert.ok(true, 'trying to use a bad selector')
-      Util.getSelectorFromElement($el[0])
-    } catch (e) {
-      assert.ok(e instanceof DOMException)
-    }
+    assert.strictEqual(Util.getSelectorFromElement($el[0]), null)
+
+    var $el2 = $('<a href="/posts"></a>').appendTo($('#qunit-fixture'))
+
+    assert.strictEqual(Util.getSelectorFromElement($el2[0]), null)
   })
 
   QUnit.test('Util.typeCheckConfig should thrown an error when a bad config is passed', function (assert) {