From 4273ca67333d0a6952c3383dff521bfda20fe8c5 Mon Sep 17 00:00:00 2001 From: Jonathan Kingston Date: Tue, 14 Mar 2017 16:48:09 +0000 Subject: [PATCH 1/3] Fix upgrading issue whilst also ensuring dead objects won't happen. #373 --- index.js | 63 ++++++++++++++++++++++---------------- package.json | 2 +- webextension/manifest.json | 2 +- 3 files changed, 38 insertions(+), 29 deletions(-) diff --git a/index.js b/index.js index 2b9a1fa..e17c655 100644 --- a/index.js +++ b/index.js @@ -119,11 +119,12 @@ const ContainerService = { _containerWasEnabled: false, _onThemeChangedCallback: null, - init(installation) { + init(installation, reason) { // If we are just been installed, we must store some information for the // uninstallation. This object contains also a version number, in case we // need to implement a migration in the future. - if (installation) { + // In 1.1.1 and less we deleted savedConfiguration on upgrade so we need to rebuild + if (installation && (reason !== "upgrade" || !ss.storage.savedConfiguration)) { let preInstalledIdentities = []; // eslint-disable-line prefer-const ContextualIdentityProxy.getIdentities().forEach(identity => { preInstalledIdentities.push(identity.userContextId); @@ -286,13 +287,17 @@ const ContainerService = { return identity; }; - this._oldGetIdentityFromId = ContextualIdentityService.getIdentityFromId; + if (!this._oldGetIdentityFromId) { + this._oldGetIdentityFromId = ContextualIdentityService.getIdentityFromId; + } ContextualIdentityService.getIdentityFromId = function(userContextId) { return this.workaroundForCookieManager(ContainerService._oldGetIdentityFromId, userContextId); }; if ("getPublicIdentityFromId" in ContextualIdentityService) { - this._oldGetPublicIdentityFromId = ContextualIdentityService.getPublicIdentityFromId; + if (!this._oldGetPublicIdentityFromId) { + this._oldGetPublicIdentityFromId = ContextualIdentityService.getPublicIdentityFromId; + } ContextualIdentityService.getPublicIdentityFromId = function(userContextId) { return this.workaroundForCookieManager(ContainerService._oldGetPublicIdentityFromId, userContextId); }; @@ -1076,7 +1081,7 @@ const ContainerService = { }, // Uninstallation - uninstall() { + uninstall(reason) { const data = ss.storage.savedConfiguration; if (!data) { throw new DOMError("ERROR - No saved configuration!!"); @@ -1086,11 +1091,13 @@ const ContainerService = { throw new DOMError("ERROR - Unknown version!!"); } - PREFS.forEach(pref => { - if (pref[0] in data.prefs) { - prefService.set(pref[0], data.prefs[pref[0]]); - } - }); + if (reason !== "upgrade") { + PREFS.forEach(pref => { + if (pref[0] in data.prefs) { + prefService.set(pref[0], data.prefs[pref[0]]); + } + }); + } // Note: We cannot go back renaming the Finance identity back to Banking: // the locale system doesn't work with renamed containers. @@ -1105,7 +1112,7 @@ const ContainerService = { // Let's close all the container tabs. // Note: We cannot use _closeTabs() because at this point tab.window is // null. - if (!this._containerWasEnabled) { + if (!this._containerWasEnabled && reason !== "upgrade") { for (let tab of window.tabs) { // eslint-disable-line prefer-const if (this._getUserContextIdFromTab(tab)) { tab.close(); @@ -1122,22 +1129,24 @@ const ContainerService = { // all the configuration must go away now. this._windowMap = new Map(); - // Let's forget all the previous closed tabs. - this._forgetIdentity(); + if (reason !== "upgrade") { + // Let's forget all the previous closed tabs. + this._forgetIdentity(); - const preInstalledIdentities = data.preInstalledIdentities; - ContextualIdentityProxy.getIdentities().forEach(identity => { - if (!preInstalledIdentities.includes(identity.userContextId)) { - ContextualIdentityProxy.remove(identity.userContextId); - } else { - // Let's cleanup all the cookies for this container. - Services.obs.notifyObservers(null, "clear-origin-attributes-data", - JSON.stringify({ userContextId: identity.userContextId })); - } - }); + const preInstalledIdentities = data.preInstalledIdentities; + ContextualIdentityProxy.getIdentities().forEach(identity => { + if (!preInstalledIdentities.includes(identity.userContextId)) { + ContextualIdentityProxy.remove(identity.userContextId); + } else { + // Let's cleanup all the cookies for this container. + Services.obs.notifyObservers(null, "clear-origin-attributes-data", + JSON.stringify({ userContextId: identity.userContextId })); + } + }); - // Let's delete the configuration. - delete ss.storage.savedConfiguration; + // Let's delete the configuration. + delete ss.storage.savedConfiguration; + } // Begin-Of-Hack if (this._oldGetIdentityFromId) { @@ -1557,7 +1566,7 @@ exports.main = function (options) { options.loadReason === "upgrade"; // Let's start :) - ContainerService.init(installation); + ContainerService.init(installation, options.loadReason); }; exports.onUnload = function (reason) { @@ -1565,6 +1574,6 @@ exports.onUnload = function (reason) { reason === "downgrade" || reason === "uninstall" || reason === "upgrade") { - ContainerService.uninstall(); + ContainerService.uninstall(reason); } }; diff --git a/package.json b/package.json index fe06cbf..d9eb4f5 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "testpilot-containers", "title": "Containers Experiment", "description": "Containers works by isolating cookie jars using separate origin-attributes defined visually by colored ‘Container Tabs’. This add-on is a modified version of the containers feature for Firefox Test Pilot.", - "version": "1.1.1", + "version": "2.0.0", "author": "Andrea Marchesini, Luke Crouch and Jonathan Kingston", "bugs": { "url": "https://github.com/mozilla/testpilot-containers/issues" diff --git a/webextension/manifest.json b/webextension/manifest.json index 8b0dee7..6341619 100644 --- a/webextension/manifest.json +++ b/webextension/manifest.json @@ -1,7 +1,7 @@ { "manifest_version": 2, "name": "Containers Experiment", - "version": "1.1.1", + "version": "2.0.0", "description": "Containers works by isolating cookie jars using separate origin-attributes defined visually by colored ‘Container Tabs’. This add-on is a modified version of the containers feature for Firefox Test Pilot.", "icons": { From 93a12df19b47e6f9cc572cac5ce8fe50c7632710 Mon Sep 17 00:00:00 2001 From: Jonathan Kingston Date: Mon, 13 Mar 2017 19:46:22 +0000 Subject: [PATCH 2/3] Adding better sanitising of strings by using textContent. Fixes #372 --- webextension/js/popup.js | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/webextension/js/popup.js b/webextension/js/popup.js index c108558..23a6d66 100644 --- a/webextension/js/popup.js +++ b/webextension/js/popup.js @@ -19,15 +19,15 @@ const P_CONTAINER_EDIT = "containerEdit"; const P_CONTAINER_DELETE = "containerDelete"; /** - * Escapes any occurances of &, ", < or > with XML entities. + * Escapes any occurances of &, ", <, > or / with XML entities. * * @param {string} str * The string to escape. * @return {string} The escaped string. */ function escapeXML(str) { - const replacements = {"&": "&", "\"": """, "'": "'", "<": "<", ">": ">"}; - return String(str).replace(/[&"''<>]/g, m => replacements[m]); + const replacements = {"&": "&", "\"": """, "'": "'", "<": "<", ">": ">", "/": "/"}; + return String(str).replace(/[&"'<>/]/g, m => replacements[m]); } /** @@ -278,7 +278,8 @@ Logic.registerPanel(P_CONTAINERS_LIST, { data-identity-color="${identity.color}"> -
${identity.name}
`; +
`; + context.querySelector(".container-name").textContent = identity.name; manage.innerHTML = ""; fragment.appendChild(tr); @@ -353,7 +354,7 @@ Logic.registerPanel(P_CONTAINER_INFO, { fragment.appendChild(incompatEl); incompatEl.setAttribute("id", "container-info-movetabs-incompat"); - incompatEl.innerText = "Incompatible with other Experiments."; + incompatEl.textContent = "Incompatible with other Experiments."; incompatEl.classList.add("container-info-tab-row"); moveTabsEl.parentNode.insertBefore(fragment, moveTabsEl.nextSibling); @@ -377,7 +378,7 @@ Logic.registerPanel(P_CONTAINER_INFO, { const identity = Logic.currentIdentity(); // Populating the panel: name and icon - document.getElementById("container-info-name").innerText = identity.name; + document.getElementById("container-info-name").textContent = identity.name; const icon = document.getElementById("container-info-icon"); icon.setAttribute("data-identity-icon", identity.image); @@ -392,7 +393,7 @@ Logic.registerPanel(P_CONTAINER_INFO, { hideShowIcon.src = identity.hasHiddenTabs ? CONTAINER_UNHIDE_SRC : CONTAINER_HIDE_SRC; const hideShowLabel = document.getElementById("container-info-hideorshow-label"); - hideShowLabel.innerText = identity.hasHiddenTabs ? "Show this container" : "Hide this container"; + hideShowLabel.textContent = identity.hasHiddenTabs ? "Show this container" : "Hide this container"; // Let's remove all the previous tabs. const table = document.getElementById("container-info-table"); @@ -466,21 +467,23 @@ Logic.registerPanel(P_CONTAINERS_EDIT, { data-identity-color="${identity.color}"> -
${identity.name}
+
`; + tr.querySelector(".container-name").textContent = identity.name; + tr.querySelector(".edit-container .pop-button-image").setAttribute("title", `Edit ${identity.name} container`); + tr.querySelector(".remove-container .pop-button-image").setAttribute("title", `Edit ${identity.name} container`); + tr.addEventListener("click", e => { if (e.target.matches(".edit-container-icon") || e.target.parentNode.matches(".edit-container-icon")) { @@ -618,7 +621,7 @@ Logic.registerPanel(P_CONTAINER_DELETE, { const identity = Logic.currentIdentity(); // Populating the panel: name and icon - document.getElementById("delete-container-name").innerText = identity.name; + document.getElementById("delete-container-name").textContent = identity.name; const icon = document.getElementById("delete-container-icon"); icon.setAttribute("data-identity-icon", identity.image); From 651b2948af0073dfca4d4ce918e16866313248fa Mon Sep 17 00:00:00 2001 From: Jonathan Kingston Date: Wed, 15 Mar 2017 06:34:31 +0000 Subject: [PATCH 3/3] Rename linter used to check for unsafe --- .eslintrc.js | 6 +++--- package.json | 2 +- webextension/js/popup.js | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 748b27b..d44e921 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -16,7 +16,7 @@ module.exports = { }, "plugins": [ "promise", - "unsafe-property-assignment" + "no-unescaped" ], "root": true, "rules": { @@ -29,8 +29,8 @@ module.exports = { "promise/no-promise-in-callback": "warn", "promise/no-return-wrap": "error", "promise/param-names": "error", - "unsafe-property-assignment/no-key-assignment": ["error"], - "unsafe-property-assignment/enforce-tagged-template-protection": ["error"], + "no-unescaped/no-key-assignment": "error", + "no-unescaped/enforce": "error", "eqeqeq": "error", "indent": ["error", 2], diff --git a/package.json b/package.json index d9eb4f5..33c1ab3 100644 --- a/package.json +++ b/package.json @@ -12,8 +12,8 @@ "addons-linter": "^0.15.14", "deploy-txp": "^1.0.7", "eslint": "^3.17.1", + "eslint-plugin-no-unescaped": "^1.1.0", "eslint-plugin-promise": "^3.4.0", - "eslint-plugin-unsafe-property-assignment": "^1.0.2", "htmllint-cli": "^0.0.5", "jpm": "^1.2.2", "npm-run-all": "^4.0.0", diff --git a/webextension/js/popup.js b/webextension/js/popup.js index 23a6d66..7ed2b50 100644 --- a/webextension/js/popup.js +++ b/webextension/js/popup.js @@ -555,7 +555,7 @@ Logic.registerPanel(P_CONTAINER_EDIT, { const colorRadioFieldset = document.getElementById("edit-container-panel-choose-color"); colors.forEach((containerColor) => { const templateInstance = document.createElement("span"); - // eslint-disable-next-line unsafe-property-assignment/enforce-tagged-template-protection + // eslint-disable-next-line no-unescaped/enforce templateInstance.innerHTML = colorRadioTemplate(containerColor); colorRadioFieldset.appendChild(templateInstance); }); @@ -568,7 +568,7 @@ Logic.registerPanel(P_CONTAINER_EDIT, { const iconRadioFieldset = document.getElementById("edit-container-panel-choose-icon"); icons.forEach((containerIcon) => { const templateInstance = document.createElement("span"); - // eslint-disable-next-line unsafe-property-assignment/enforce-tagged-template-protection + // eslint-disable-next-line no-unescaped/enforce templateInstance.innerHTML = iconRadioTemplate(containerIcon); iconRadioFieldset.appendChild(templateInstance); });