Refactor the 'action' and 'formAction' properties of the 'Code instru…

…mentation' subsystem (#1562)

* Refactor the 'action' and 'formAction' properties of the 'Code instrumentation' subsystem

* fix tests

* remove unnecessary code

package.json

@@ -50,7 +50,7 @@
     "del": "^0.1.2",
     "endpoint-utils": "^1.0.1",
     "eslint": "4.17.0",
-    "eslint-plugin-hammerhead": "0.1.6",
+    "eslint-plugin-hammerhead": "0.1.7",
     "express": "3.2.5",
     "express-ntlm": "2.1.5",
     "gulp": "^3.8.7",

src/client/page-navigation-watch.js

@@ -24,7 +24,7 @@ export default class PageNavigationWatch extends EventEmiter {
         const onFormSubmit = form => {
             const targetWindow = PageNavigationWatch._getTargetWindow(form);
 
-            PageNavigationWatch._onNavigationTriggeredInWindow(targetWindow, form.action);
+            PageNavigationWatch._onNavigationTriggeredInWindow(targetWindow, nativeMethods.formActionGetter.call(form));
         };
 
         // NOTE: fires when form.submit() is called

src/client/sandbox/code-instrumentation/properties/index.js

@@ -7,17 +7,15 @@ import * as destLocation from '../../../utils/destination-location';
 import * as domUtils from '../../../utils/dom';
 import * as typeUtils from '../../../utils/types';
 import * as urlUtils from '../../../utils/url';
-import { ensureOriginTrailingSlash, HASH_RE } from '../../../../utils/url';
+import { ensureOriginTrailingSlash } from '../../../../utils/url';
 import { cleanUpHtml, processHtml } from '../../../utils/html';
 import { getAttributesProperty } from './attributes';
-import domProcessor from '../../../dom-processor';
 import styleProcessor from '../../../../processing/style';
 import { processScript } from '../../../../processing/script';
 import { remove as removeProcessingHeader } from '../../../../processing/script/header';
 import INSTRUCTION from '../../../../processing/script/instruction';
 import { shouldInstrumentProperty } from '../../../../processing/script/instrumented';
 import nativeMethods from '../../native-methods';
-import { emptyActionAttrFallbacksToTheLocation } from '../../../utils/feature-detection';
 import DOMMutationTracker from '../../node/live-node-list/dom-mutation-tracker';
 import { isJsProtocol, processJsAttrValue } from '../../../../processing/dom';
 
@@ -49,21 +47,6 @@ export default class PropertyAccessorsInstrumentation extends SandboxBase {
         throw new Error(msg);
     }
 
-    static _getUrlAttr (el, attr) {
-        const attrValue = nativeMethods.getAttribute.call(el, attr);
-
-        if (attrValue === '' || attrValue === null && attr === 'action' && emptyActionAttrFallbacksToTheLocation)
-            return destLocation.get();
-
-        else if (attrValue === null)
-            return '';
-
-        else if (HASH_RE.test(attrValue))
-            return destLocation.withHash(attrValue);
-
-        return urlUtils.resolveUrlAsDest(attrValue);
-    }
-
     static _setTextProp (el, propName, text) {
         const processedText = text !== null && text !== void 0 ? String(text) : text;
 
@@ -109,22 +92,6 @@ export default class PropertyAccessorsInstrumentation extends SandboxBase {
 
     _createPropertyAccessors (window, document) {
         return {
-            action: {
-                condition: el => domUtils.isDomElement(el) && domProcessor.isUrlAttr(el, 'action'),
-
-                get: el => {
-                    if (domUtils.isDomElement(el.action))
-                        return el.action;
-
-                    return PropertyAccessorsInstrumentation._getUrlAttr(el, 'action');
-                },
-                set: (el, value) => {
-                    this.elementSandbox.setAttributeCore(el, ['action', value]);
-
-                    return value;
-                }
-            },
-
             attributes: {
                 condition: el => domUtils.isDomElement(el) && el.attributes instanceof window.NamedNodeMap,
 
@@ -151,17 +118,6 @@ export default class PropertyAccessorsInstrumentation extends SandboxBase {
                 set:       () => void 0
             },
 
-            formAction: {
-                condition: el => domUtils.isDomElement(el) && domProcessor.isUrlAttr(el, 'formaction'),
-
-                get: el => PropertyAccessorsInstrumentation._getUrlAttr(el, 'formaction'),
-                set: (el, value) => {
-                    this.elementSandbox.setAttributeCore(el, ['formaction', value]);
-
-                    return value;
-                }
-            },
-
             href: {
                 condition: LocationAccessorsInstrumentation.isLocationWrapper,
 

src/client/sandbox/native-methods.js

@@ -256,6 +256,9 @@ class NativeMethods {
         const svgAnimStrAnimValDescriptor = win.Object.getOwnPropertyDescriptor(win.SVGAnimatedString.prototype, 'animVal');
         const svgAnimStrBaseValDescriptor = win.Object.getOwnPropertyDescriptor(win.SVGAnimatedString.prototype, 'baseVal');
         const inputAutocompleteDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLInputElement.prototype, 'autocomplete');
+        const formActionDescriptor        = win.Object.getOwnPropertyDescriptor(win.HTMLFormElement.prototype, 'action');
+        const inputFormActionDescriptor   = win.Object.getOwnPropertyDescriptor(win.HTMLInputElement.prototype, 'formAction');
+        const buttonFormActionDescriptor  = win.Object.getOwnPropertyDescriptor(win.HTMLButtonElement.prototype, 'formAction');
 
         // Setters
         this.objectDataSetter        = objectDataDescriptor.set;
@@ -285,6 +288,9 @@ class NativeMethods {
         this.baseTargetSetter        = baseTargetDescriptor.set;
         this.svgAnimStrBaseValSetter = svgAnimStrBaseValDescriptor.set;
         this.inputAutocompleteSetter = inputAutocompleteDescriptor.set;
+        this.formActionSetter        = formActionDescriptor.set;
+        this.inputFormActionSetter   = inputFormActionDescriptor.set;
+        this.buttonFormActionSetter  = buttonFormActionDescriptor.set;
 
         // NOTE: Event properties is located in window prototype only in IE11
         this.isEventPropsLocatedInProto = win.Window.prototype.hasOwnProperty('onerror');
@@ -345,6 +351,9 @@ class NativeMethods {
         this.svgAnimStrAnimValGetter        = svgAnimStrAnimValDescriptor.get;
         this.svgAnimStrBaseValGetter        = svgAnimStrBaseValDescriptor.get;
         this.inputAutocompleteGetter        = inputAutocompleteDescriptor.get;
+        this.formActionGetter               = formActionDescriptor.get;
+        this.inputFormActionGetter          = inputFormActionDescriptor.get;
+        this.buttonFormActionGetter         = buttonFormActionDescriptor.get;
 
         const anchorOriginDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLAnchorElement.prototype, 'origin');
 

src/client/sandbox/node/element.js

@@ -142,7 +142,7 @@ export default class ElementSandbox extends SandboxBase {
                         resourceType = 'f';
 
                         if (el.form) {
-                            const parsedFormAction = urlUtils.parseProxyUrl(el.form.action);
+                            const parsedFormAction = urlUtils.parseProxyUrl(nativeMethods.formActionGetter.call(el.form));
 
                             if (parsedFormAction)
                                 resourceType = parsedFormAction.resourceType;

src/client/sandbox/node/window.js

@@ -697,6 +697,13 @@ export default class WindowSandbox extends SandboxBase {
             window.HTMLIFrameElement
         ]);
 
+        this._overrideUrlAttrDescriptors('action', [window.HTMLFormElement]);
+
+        this._overrideUrlAttrDescriptors('formAction', [
+            window.HTMLInputElement,
+            window.HTMLButtonElement
+        ]);
+
         this._overrideUrlAttrDescriptors('href', [
             window.HTMLAnchorElement,
             window.HTMLLinkElement,

src/client/utils/feature-detection.js

@@ -1,9 +1,11 @@
 import nativeMethods from '../sandbox/native-methods';
 import * as browserUtils from './browser';
 
+const form = nativeMethods.createElement.call(document, 'form');
+
 // NOTE: In some browsers, elements without the url attribute return the location url
 // when accessing this attribute directly. See form.action in Edge 25 as an example.
-export const emptyActionAttrFallbacksToTheLocation = nativeMethods.createElement.call(document, 'form').action ===
+export const emptyActionAttrFallbacksToTheLocation = nativeMethods.formActionGetter.call(form) ===
                                                      window.location.toString();
 
 // NOTE: In Chrome, toString(window) equals '[object Window]' and toString(Window.prototype) equals '[object Blob]',

src/processing/script/instrumented.js

@@ -11,12 +11,10 @@ export const METHODS = [
 ];
 
 export const PROPERTIES = [
-    'action',
     'attributes',
     'data',
     'firstChild',
     'firstElementChild',
-    'formAction',
     'href',
     'innerHTML',
     'innerText',

test/client/fixtures/sandbox/code-instrumentation/getters-test.js

@@ -118,8 +118,8 @@ test('input.formaction, button.formaction', function () {
     var button = document.createElement('button');
     var input  = document.createElement('input');
 
-    setProperty(button, 'formAction', './button.html');
-    setProperty(input, 'formAction', './input.html');
+    button.formAction = './button.html';
+    input.formAction  = './input.html';
 
     strictEqual(nativeMethods.getAttribute.call(button, 'formaction'), urlUtils.getProxyUrl('./button.html', { resourceType: 'f' }));
     strictEqual(nativeMethods.getAttribute.call(input, 'formaction'), urlUtils.getProxyUrl('./input.html', { resourceType: 'f' }));
@@ -420,35 +420,6 @@ test('we should not process element\'s properties if they do not exist (GH-1164)
     });
 });
 
-test('form.action should return element when the form contains element with the "action" attribute name (GH-1291)', function () {
-    var form        = document.createElement('form');
-    var input       = document.createElement('input');
-    var nativeForm  = nativeMethods.createElement.call(document, 'form');
-    var nativeInput = nativeMethods.createElement.call(document, 'input');
-
-    form.setAttribute('action', 'http://example.com/test1');
-    input.setAttribute('name', 'action');
-    nativeMethods.setAttribute.call(nativeForm, 'action', 'http://example.com/test1');
-    nativeMethods.setAttribute.call(nativeInput, 'name', 'action');
-
-    strictEqual(getProperty(form, 'action'), nativeForm.action);
-
-    form.appendChild(input);
-    nativeForm.appendChild(nativeInput);
-
-    strictEqual(getProperty(form, 'action').tagName, nativeForm.action.tagName);
-
-    setProperty(form, 'action', 'http://example.com/test2');
-    nativeForm.action = 'http://example.com/test2';
-
-    strictEqual(getProperty(form, 'action').tagName, nativeForm.action.tagName);
-
-    form.removeChild(input);
-    nativeForm.removeChild(nativeInput);
-
-    strictEqual(getProperty(form, 'action'), nativeForm.action);
-});
-
 test('getProperty function should not throw an error for document.all property (GH-1046)', function () {
     try {
         strictEqual(getProperty(document.all, 0), document.documentElement);

test/client/fixtures/sandbox/node/attributes-test.js

@@ -53,78 +53,6 @@ if (!browserUtils.isIE || browserUtils.version !== 11) {
     });
 }
 
-test('url', function () {
-    var testUrlAttr = function (tagName, attr) {
-        var el         = nativeMethods.createElement.call(document, tagName);
-        var storedAttr = DomProcessor.getStoredAttrName(attr);
-        var namespace  = 'http://www.w3.org/1999/xhtml';
-
-        var getAttr = function () {
-            return nativeMethods.getAttribute.call(el, attr);
-        };
-
-        var getWrapAttr = function () {
-            return nativeMethods.getAttribute.call(el, storedAttr);
-        };
-
-        el.setAttribute(attr, '');
-
-        var emptyAttrValue = el[attr];
-        var dest           = 'http://dest.com/';
-        var resourceType   = null;
-
-        if (tagName === 'script')
-            resourceType = 's';
-        else if (tagName === 'form' || attr === 'formAction')
-            resourceType = 'f';
-
-        var proxy = urlUtils.getProxyUrl(dest, { resourceType: resourceType });
-
-        setProperty(el, attr, dest);
-        strictEqual(el[attr], proxy);
-        strictEqual(getProperty(el, attr), dest);
-        strictEqual(getAttr(), proxy);
-        strictEqual(getWrapAttr(), dest);
-
-        var newUrl      = '/image';
-        var proxyNewUrl = urlUtils.getProxyUrl('/image', { resourceType: resourceType });
-
-        el.setAttribute(attr, newUrl);
-        strictEqual(el[attr], proxyNewUrl);
-        strictEqual(getProperty(el, attr), urlUtils.parseProxyUrl(proxyNewUrl).destUrl);
-        strictEqual(getAttr(), proxyNewUrl);
-        strictEqual(getWrapAttr(), newUrl);
-
-        setProperty(el, attr, '');
-        strictEqual(getWrapAttr(), '');
-        strictEqual(getAttr(), '');
-        strictEqual(el[attr], emptyAttrValue);
-        strictEqual(getProperty(el, attr), destLocation.get());
-
-        el.removeAttribute(attr);
-        strictEqual(getWrapAttr(), null);
-        strictEqual(getAttr(), null);
-
-        if (attr === 'action' && featureDetection.emptyActionAttrFallbacksToTheLocation)
-            strictEqual(getProperty(el, attr), destLocation.get());
-        else
-            strictEqual(getProperty(el, attr), '');
-
-        el.setAttributeNS(namespace, attr, dest);
-        strictEqual(nativeMethods.getAttributeNS.call(el, namespace, attr), proxy);
-        strictEqual(nativeMethods.getAttributeNS.call(el, namespace, storedAttr), dest);
-    };
-
-    var testData = [
-        { tagName: 'form', attr: 'action' },
-        { tagName: 'input', attr: 'formAction' },
-        { tagName: 'button', attr: 'formAction' }
-    ];
-
-    for (var i = 0; i < testData.length; i++)
-        testUrlAttr(testData[i].tagName, testData[i].attr);
-});
-
 test('url attributes overridden by descriptor', function () {
     var testUrlAttr = function (tagName, attr, getter) {
         var el         = document.createElement(tagName);
@@ -194,7 +122,10 @@ test('url attributes overridden by descriptor', function () {
         { tagName: 'script', attr: 'src', getter: nativeMethods.scriptSrcGetter },
         { tagName: 'embed', attr: 'src', getter: nativeMethods.embedSrcGetter },
         { tagName: 'a', attr: 'href', getter: nativeMethods.anchorHrefGetter },
-        { tagName: 'link', attr: 'href', getter: nativeMethods.linkHrefGetter }
+        { tagName: 'link', attr: 'href', getter: nativeMethods.linkHrefGetter },
+        { tagName: 'form', attr: 'action', getter: nativeMethods.formActionGetter },
+        { tagName: 'input', attr: 'formAction', getter: nativeMethods.inputFormActionGetter },
+        { tagName: 'button', attr: 'formAction', getter: nativeMethods.buttonFormActionGetter }
     ];
 
     if (nativeMethods.htmlManifestGetter)
@@ -218,14 +149,14 @@ if (!browserUtils.isIE || browserUtils.version > 9) {
         nativeMethods.setAttribute.call(form, 'action', urlUtils.getProxyUrl('./action.html', { resourceType: 'f' }));
         input.setAttribute('formaction', './input.html');
         button.setAttribute('formaction', './button.html');
-        strictEqual(urlUtils.parseProxyUrl(input.formAction).resourceType, 'f');
-        strictEqual(urlUtils.parseProxyUrl(button.formAction).resourceType, 'f');
+        strictEqual(urlUtils.parseProxyUrl(nativeMethods.inputFormActionGetter.call(input)).resourceType, 'f');
+        strictEqual(urlUtils.parseProxyUrl(nativeMethods.buttonFormActionGetter.call(button)).resourceType, 'f');
 
         nativeMethods.setAttribute.call(form, 'action', urlUtils.getProxyUrl('./action.html', { resourceType: 'fi' }));
         input.setAttribute('formaction', './input.html');
         button.setAttribute('formaction', './button.html');
-        strictEqual(urlUtils.parseProxyUrl(input.formAction).resourceType, 'fi');
-        strictEqual(urlUtils.parseProxyUrl(button.formAction).resourceType, 'fi');
+        strictEqual(urlUtils.parseProxyUrl(nativeMethods.inputFormActionGetter.call(input)).resourceType, 'fi');
+        strictEqual(urlUtils.parseProxyUrl(nativeMethods.buttonFormActionGetter.call(button)).resourceType, 'fi');
     });
 }
 

test/client/fixtures/sandbox/node/dom-processor-test.js

@@ -362,13 +362,13 @@ test('add element with `formaction` tag to the form', function () {
     var form  = document.createElement('form');
     var input = document.createElement('input');
 
-    form.action = urlUtils.getProxyUrl('./form.html', { resourceType: 'if' });
+    nativeMethods.formActionSetter.call(form, urlUtils.getProxyUrl('./form.html', { resourceType: 'if' }));
 
     input.setAttribute('formAction', './input.html');
-    strictEqual(input.formAction, urlUtils.getProxyUrl('./input.html', { resourceType: 'f' }));
+    strictEqual(nativeMethods.inputFormActionGetter.call(input), urlUtils.getProxyUrl('./input.html', { resourceType: 'f' }));
 
     form.appendChild(input);
-    strictEqual(input.formAction, urlUtils.getProxyUrl('./input.html', { resourceType: 'if' }));
+    strictEqual(nativeMethods.inputFormActionGetter.call(input), urlUtils.getProxyUrl('./input.html', { resourceType: 'if' }));
 });
 
 
@@ -672,7 +672,7 @@ test('should reprocess tags that doesn\'t processed on server side (GH-838)', fu
             var processedAnchor     = iframe.contentDocument.querySelector('#processed-anchor');
             var processedForm       = iframe.contentDocument.querySelector('#processed-form');
             var processedAnchorHref = nativeMethods.anchorHrefGetter.call(processedAnchor);
-            var processedFormAction = processedForm.action;
+            var processedFormAction = nativeMethods.formActionGetter.call(processedForm);
 
             strictEqual(processedAnchorHref, urlUtils.getProxyUrl('http://localhost/anchor-action.html'));
             strictEqual(processedFormAction, urlUtils.getProxyUrl('http://localhost/form-action.html', { resourceType: 'f' }));

test/client/fixtures/target-attr-test.js

@@ -289,9 +289,9 @@ test('The form in the iframe (GH-880)', function () {
                 return false;
             };
 
-            strictEqual(urlUtils.parseProxyUrl(form.action).resourceType, 'if');
+            strictEqual(urlUtils.parseProxyUrl(nativeMethods.formActionGetter.call(form)).resourceType, 'if');
             form.submit();
-            strictEqual(urlUtils.parseProxyUrl(form.action).resourceType, 'if');
+            strictEqual(urlUtils.parseProxyUrl(nativeMethods.formActionGetter.call(form)).resourceType, 'if');
         });
 });