From e848b09d4baafc6938ea99814c8830db66f5e822 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20H=C3=A4usele?= Date: Fri, 17 Mar 2023 11:32:54 +0100 Subject: [PATCH 1/5] WIP --- LTS.md | 34 ++++++++++++++++++++++++ script/run_tests | 38 +++++++++++++++++++++++++++ src/rails.js | 25 ++++++++++++++++++ test/public/test/data-disable-with.js | 24 +++++++++++++++++ test/public/test/data-method.js | 19 ++++++++++++++ test/public/test/data-remote.js | 26 ++++++++++++++++++ 6 files changed, 166 insertions(+) create mode 100644 LTS.md create mode 100755 script/run_tests diff --git a/LTS.md b/LTS.md new file mode 100644 index 00000000..e1487f41 --- /dev/null +++ b/LTS.md @@ -0,0 +1,34 @@ +# LTS + +This is the LTS fork of jquery-ujs. + +## Supported jQuery versions + +We test our patches against the following jQuery versions: + +* 1.8.0 +* 1.8.1 +* 1.8.2 +* 1.8.3 +* 1.9.0 +* 1.9.1 +* 1.10.0 +* 1.10.1 +* 1.10.2 +* 1.11.0 +* 2.0.0 +* 2.1.0 +* 3.0.0 + +## Running tests + +``` +# Runs all Qunit tests in all versions in chrome with capybara +$script/run_tests + +# Runs Qunit tests for 1.8.0 & 1.8.1 +$VERSIONS=1.8.0,1.8.1 script/run_tests + +# Runs suite with :selenium-chrome +$NO_HEADLESS=1 script/run_tests +``` diff --git a/script/run_tests b/script/run_tests new file mode 100755 index 00000000..0b80bb54 --- /dev/null +++ b/script/run_tests @@ -0,0 +1,38 @@ +#!/usr/bin/env ruby +$LOAD_PATH.unshift File.expand_path('../../test', __FILE__) + +ENV['APP_ENV'] = 'test' + +require "capybara" +require "capybara/dsl" +require "capybara/minitest" +require "minitest/autorun" +require "byebug" +require "server" + +class TestRunner < Minitest::Test + include Capybara::DSL + include Capybara::Minitest::Assertions + + Capybara.default_driver = !!ENV['NO_HEADLESS'] ? :selenium_chrome : :selenium_chrome_headless + Capybara.server = :puma, { Silent: true } + + def setup + Capybara.app = Sinatra::Application.new + end + + if ENV.fetch("VERSIONS", '').split(',').count > 0 + ALL_TESTED_VERSIONS = ENV["VERSIONS"].split(',') + raise "You're trying to test against a unsupported version" if (ALL_TESTED_VERSIONS - JQUERY_VERSIONS).count > 0 + else + ALL_TESTED_VERSIONS = JQUERY_VERSIONS.dup + end + + ALL_TESTED_VERSIONS.each do |version| + define_method("test_jquery_#{version}_suite_passes") do + visit "/?version=#{version}" + assert_no_selector "#qunit-banner.qunit-fail", wait: 20 + assert_selector "#qunit-banner.qunit-pass", wait: 20 + end + end +end diff --git a/src/rails.js b/src/rails.js index 0e27110d..75346a73 100644 --- a/src/rails.js +++ b/src/rails.js @@ -64,6 +64,22 @@ return $('meta[name=csrf-token]').attr('content'); }, + // Checks if element is contentEditable + isContentEditable: function(element) { + var isEditable = false + while (true) { + if (element[0].isContentEditable) { + isEditable = true + break + } + element = element.parent() + if (!element[0]) { + break + } + } + return isEditable + }, + // URL param that must contain the CSRF token csrfParam: function() { return $('meta[name=csrf-param]').attr('content'); @@ -221,6 +237,10 @@ form = $('
'), metadataInput = ''; + if (rails.isContentEditable(link)) { + return + } + if (csrfParam !== undefined && csrfToken !== undefined && !rails.isCrossDomain(href)) { metadataInput += ''; } @@ -244,6 +264,9 @@ - Sets disabled property to true */ disableFormElements: function(form) { + if (rails.isContentEditable(form)) { + return + } rails.formElements(form, rails.disableSelector).each(function() { rails.disableFormElement($(this)); }); @@ -431,6 +454,8 @@ var link = $(this), method = link.data('method'), data = link.data('params'), metaClick = e.metaKey || e.ctrlKey; if (!rails.allowAction(link)) return rails.stopEverything(e); + if (rails.isContentEditable(link)) { return false; } + if (!metaClick && link.is(rails.linkDisableSelector)) rails.disableElement(link); if (rails.isRemote(link)) { diff --git a/test/public/test/data-disable-with.js b/test/public/test/data-disable-with.js index 1bb67426..2b3fa25c 100644 --- a/test/public/test/data-disable-with.js +++ b/test/public/test/data-disable-with.js @@ -117,6 +117,30 @@ asyncTest('form input[type=submit][data-disable-with] disables', 6, function(){ }, 30); }); +asyncTest('form button with "data-disable-with" attribute and contenteditable is not modified', 6, function() { + $("#qunit-fixture").replaceWith(` +
+
+
+ +
+
+
+ `) + + var button = $('#qunit-fixture').find('button') + var form = $('#qunit-fixture').find('form') + App.checkEnabledState(button, 'Submit') + + setTimeout(function() { + App.checkEnabledState(button, 'Submit') + start() + }, 13) + form.trigger('submit') + + App.checkEnabledState(button, 'Submit'); +}); + test('form input[type=submit][data-disable-with] re-enables when `pageshow` event is triggered', function(){ var form = $('form:not([data-remote])'), input = form.find('input[type=submit]'); diff --git a/test/public/test/data-method.js b/test/public/test/data-method.js index 32b90d70..8483149a 100644 --- a/test/public/test/data-method.js +++ b/test/public/test/data-method.js @@ -29,6 +29,25 @@ asyncTest('link with "data-method" set to "delete"', 3, function() { }); }); +asyncTest('do not interact with contenteditable elements', 0, function() { + $('#qunit-fixture') + .replaceWith(` +
+
+ link +
+
` + ) + + $(document).on('iframe:loaded', function(e, data) { + ok(false, 'Should not trigger a request because of contenteditable parent') + }); + + $('#qunit-fixture').find('a').trigger('click'); + + setTimeout(function(){ start(); }, 50); +}) + asyncTest('link with "data-method" and CSRF', 1, function() { $('#qunit-fixture') .append('') diff --git a/test/public/test/data-remote.js b/test/public/test/data-remote.js index 215d8217..8fd82eda 100644 --- a/test/public/test/data-remote.js +++ b/test/public/test/data-remote.js @@ -54,6 +54,32 @@ asyncTest('ctrl-clicking on a link does not fire ajaxyness', 0, function() { setTimeout(function(){ start(); }, 13); }); +asyncTest('clicking on a contenteditable link does not fire ajaxyness', 0, function() { + $("#qunit-fixture").replaceWith(` +
+
+ my address +
+
+ `) + var contentEditable = $('div[contenteditable]') + var link = $('a[data-remote]'); + var e; + + contentEditable.append(link) + + link + .removeAttr('data-params') + .on('ajax:beforeSend', function() { + ok(false, 'ajax should not be triggered'); + }); + + e = $.Event('click'); + link.trigger(e); + + setTimeout(function(){ start(); }, 13); +}); + asyncTest('ctrl-clicking on a link still fires ajax for non-GET links and for links with "data-params"', 2, function() { var link = $('a[data-remote]'), e; e = $.Event('click'); From f1662dd72007b52cd907f44ca90dd9754f6ef36a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20H=C3=A4usele?= Date: Mon, 20 Mar 2023 15:47:02 +0100 Subject: [PATCH 2/5] Review suggestions: * Add the guard clause to the relevent low-level methods. --- src/rails.js | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/rails.js b/src/rails.js index 75346a73..685e48ea 100644 --- a/src/rails.js +++ b/src/rails.js @@ -127,6 +127,10 @@ handleRemote: function(element) { var method, url, data, withCredentials, dataType, options; + if (rails.isContentEditable(element)) { + return; + } + if (rails.fire(element, 'ajax:before')) { withCredentials = element.data('with-credentials') || null; dataType = element.data('type') || ($.ajaxSettings && $.ajaxSettings.dataType); @@ -265,7 +269,7 @@ */ disableFormElements: function(form) { if (rails.isContentEditable(form)) { - return + return; } rails.formElements(form, rails.disableSelector).each(function() { rails.disableFormElement($(this)); @@ -275,6 +279,10 @@ disableFormElement: function(element) { var method, replacement; + if (rails.isContentEditable(element)) { + return; + } + method = element.is('button') ? 'html' : 'val'; replacement = element.data('disable-with'); @@ -292,12 +300,20 @@ - Sets disabled property to false */ enableFormElements: function(form) { + if (rails.isContentEditable(form)) { + return; + } + rails.formElements(form, rails.enableSelector).each(function() { rails.enableFormElement($(this)); }); }, enableFormElement: function(element) { + if (rails.isContentEditable(element)) { + return; + } + var method = element.is('button') ? 'html' : 'val'; if (element.data('ujs:enable-with') !== undefined) { element[method](element.data('ujs:enable-with')); @@ -391,6 +407,10 @@ // Replace element's html with the 'data-disable-with' after storing original html // and prevent clicking on it disableElement: function(element) { + if (rails.isContentEditable(element)) { + return; + } + var replacement = element.data('disable-with'); if (replacement !== undefined) { @@ -406,6 +426,10 @@ // Restore element to its original state which was disabled by 'disableElement' above enableElement: function(element) { + if (rails.isContentEditable(element)) { + return; + } + if (element.data('ujs:enable-with') !== undefined) { element.html(element.data('ujs:enable-with')); // set to old enabled state element.removeData('ujs:enable-with'); // clean up cache @@ -481,6 +505,8 @@ if (!rails.allowAction(button) || !rails.isRemote(button)) return rails.stopEverything(e); + if (rails.isContentEditable(button)) { return false; } + if (button.is(rails.buttonDisableSelector)) rails.disableFormElement(button); var handleRemote = rails.handleRemote(button); @@ -497,6 +523,8 @@ var link = $(this); if (!rails.allowAction(link) || !rails.isRemote(link)) return rails.stopEverything(e); + if (rails.isContentEditable(link)) { return false; } + rails.handleRemote(link); return false; }); @@ -509,6 +537,8 @@ if (!rails.allowAction(form)) return rails.stopEverything(e); + if (rails.isContentEditable(form)) { return false; } + // Skip other logic when required values are missing or file upload is present if (form.attr('novalidate') === undefined) { if (form.data('ujs:formnovalidate-button') === undefined) { @@ -551,6 +581,8 @@ if (!rails.allowAction(button)) return rails.stopEverything(event); + if (rails.isContentEditable(button)) { return false; } + // Register the pressed submit button var name = button.attr('name'), data = name ? {name:name, value:button.val()} : null; From 65a8d440e195c1ba8fee4c82eb45f310e03d0c54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20H=C3=A4usele?= Date: Mon, 20 Mar 2023 15:54:00 +0100 Subject: [PATCH 3/5] Adapt codestyle --- src/rails.js | 43 ++++++++++++++----------------------------- 1 file changed, 14 insertions(+), 29 deletions(-) diff --git a/src/rails.js b/src/rails.js index 685e48ea..f66fca51 100644 --- a/src/rails.js +++ b/src/rails.js @@ -127,9 +127,7 @@ handleRemote: function(element) { var method, url, data, withCredentials, dataType, options; - if (rails.isContentEditable(element)) { - return; - } + if (rails.isContentEditable(element)) return; if (rails.fire(element, 'ajax:before')) { withCredentials = element.data('with-credentials') || null; @@ -241,9 +239,7 @@ form = $('
'), metadataInput = ''; - if (rails.isContentEditable(link)) { - return - } + if (rails.isContentEditable(link)) return; if (csrfParam !== undefined && csrfToken !== undefined && !rails.isCrossDomain(href)) { metadataInput += ''; @@ -268,9 +264,8 @@ - Sets disabled property to true */ disableFormElements: function(form) { - if (rails.isContentEditable(form)) { - return; - } + if (rails.isContentEditable(form)) return; + rails.formElements(form, rails.disableSelector).each(function() { rails.disableFormElement($(this)); }); @@ -279,9 +274,7 @@ disableFormElement: function(element) { var method, replacement; - if (rails.isContentEditable(element)) { - return; - } + if (rails.isContentEditable(element)) return; method = element.is('button') ? 'html' : 'val'; replacement = element.data('disable-with'); @@ -300,9 +293,7 @@ - Sets disabled property to false */ enableFormElements: function(form) { - if (rails.isContentEditable(form)) { - return; - } + if (rails.isContentEditable(form)) return; rails.formElements(form, rails.enableSelector).each(function() { rails.enableFormElement($(this)); @@ -310,9 +301,7 @@ }, enableFormElement: function(element) { - if (rails.isContentEditable(element)) { - return; - } + if (rails.isContentEditable(element)) return; var method = element.is('button') ? 'html' : 'val'; if (element.data('ujs:enable-with') !== undefined) { @@ -407,9 +396,7 @@ // Replace element's html with the 'data-disable-with' after storing original html // and prevent clicking on it disableElement: function(element) { - if (rails.isContentEditable(element)) { - return; - } + if (rails.isContentEditable(element)) return; var replacement = element.data('disable-with'); @@ -426,9 +413,7 @@ // Restore element to its original state which was disabled by 'disableElement' above enableElement: function(element) { - if (rails.isContentEditable(element)) { - return; - } + if (rails.isContentEditable(element)) return; if (element.data('ujs:enable-with') !== undefined) { element.html(element.data('ujs:enable-with')); // set to old enabled state @@ -478,7 +463,7 @@ var link = $(this), method = link.data('method'), data = link.data('params'), metaClick = e.metaKey || e.ctrlKey; if (!rails.allowAction(link)) return rails.stopEverything(e); - if (rails.isContentEditable(link)) { return false; } + if (rails.isContentEditable(link)) return false; if (!metaClick && link.is(rails.linkDisableSelector)) rails.disableElement(link); @@ -505,7 +490,7 @@ if (!rails.allowAction(button) || !rails.isRemote(button)) return rails.stopEverything(e); - if (rails.isContentEditable(button)) { return false; } + if (rails.isContentEditable(button)) return false; if (button.is(rails.buttonDisableSelector)) rails.disableFormElement(button); @@ -523,7 +508,7 @@ var link = $(this); if (!rails.allowAction(link) || !rails.isRemote(link)) return rails.stopEverything(e); - if (rails.isContentEditable(link)) { return false; } + if (rails.isContentEditable(link)) return false; rails.handleRemote(link); return false; @@ -537,7 +522,7 @@ if (!rails.allowAction(form)) return rails.stopEverything(e); - if (rails.isContentEditable(form)) { return false; } + if (rails.isContentEditable(form)) return false; // Skip other logic when required values are missing or file upload is present if (form.attr('novalidate') === undefined) { @@ -581,7 +566,7 @@ if (!rails.allowAction(button)) return rails.stopEverything(event); - if (rails.isContentEditable(button)) { return false; } + if (rails.isContentEditable(button)) return false; // Register the pressed submit button var name = button.attr('name'), From 5c4d9ca3924a4e4de09e962333807ac32346b984 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20H=C3=A4usele?= Date: Wed, 22 Mar 2023 13:11:31 +0100 Subject: [PATCH 4/5] Add release instructions --- .gitignore | 1 + .npmignore | 1 + LTS.md | 9 +++++++++ README.md | 4 ++++ RELEASE.md | 4 ++++ package.json | 4 ++-- 6 files changed, 21 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 251f2cfc..5ceb6ad9 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ node_modules .bundle .DS_Store *~ +.byebug_history diff --git a/.npmignore b/.npmignore index 3af83847..52e471b1 100644 --- a/.npmignore +++ b/.npmignore @@ -2,3 +2,4 @@ script/ test/ Rakefile Gemfile* +.byebug_history diff --git a/LTS.md b/LTS.md index e1487f41..2b01ec78 100644 --- a/LTS.md +++ b/LTS.md @@ -32,3 +32,12 @@ $VERSIONS=1.8.0,1.8.1 script/run_tests # Runs suite with :selenium-chrome $NO_HEADLESS=1 script/run_tests ``` + +## Releasing a new patch version + +Running `npm version patch -m 'Upgrade to %s for CVE-1234'` will: + +1. Execute all tests +2. Create a patch version bump commit +3. Push the commit +4. Publish the package diff --git a/README.md b/README.md index 2e429bd1..5205e203 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,7 @@ +# Rails-LTS fork + +Please see [LTS](LTS) for details. + Unobtrusive scripting adapter for jQuery ======================================== diff --git a/RELEASE.md b/RELEASE.md index 59316f77..e2dd5526 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -1,3 +1,7 @@ +# Rails LTS fork + +Please see [LTS](LTS) for LTS release details. + ## Releasing jquery-ujs ### Releasing to npm diff --git a/package.json b/package.json index 9299d5f5..50541952 100644 --- a/package.json +++ b/package.json @@ -1,10 +1,10 @@ { - "name": "jquery-ujs", + "name": "@railslts/jquery-ujs", "version": "1.2.3", "description": "Unobtrusive scripting adapter for jQuery", "main": "src/rails.js", "scripts": { - "test": "echo \"See the wiki: https://github.com/rails/jquery-ujs/wiki/Running-Tests-and-Contributing\" && exit 1", + "preversion": "./script/run_tests", "postversion": "git push && git push --tags && npm publish" }, "repository": { From 273cafb164b0524ffdf43fba15fc73934e693595 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20H=C3=A4usele?= Date: Wed, 22 Mar 2023 17:15:37 +0100 Subject: [PATCH 5/5] Upgrade to 1.2.4 for [CVE-2023-23913] --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 50541952..1055a5e7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@railslts/jquery-ujs", - "version": "1.2.3", + "version": "1.2.4", "description": "Unobtrusive scripting adapter for jQuery", "main": "src/rails.js", "scripts": {