fix `Cookie processing problem related to applications running on 'lo…

…calhost' with non-default port` (close #1491) (#1563)

* fix `Cookie processing problem related to applications running on 'localhost' with non-default port`

* refactor default port omitting

* fix omitDefaultPort

* localhost/127.0.0.1 case

* non-default localhost/127.0.0.1 (http) case

* add location test for default port omitting

* requested changes

* rename test function

* add tests

* rename tests

* requested changes

src/client/index.js

@@ -13,7 +13,7 @@ import * as typeUtils from './utils/types';
 import * as positionUtils from './utils/position';
 import * as styleUtils from './utils/style';
 import trim from '../utils/string-trim';
-import { isRelativeUrl, parseProxyUrl, isSpecialPage, ensureOriginTrailingSlash } from '../utils/url';
+import { isRelativeUrl, parseProxyUrl, isSpecialPage, prepareUrl } from '../utils/url';
 import * as urlUtils from './utils/url';
 import * as featureDetection from './utils/feature-detection';
 import * as htmlUtils from './utils/html';
@@ -201,7 +201,7 @@ class Hammerhead {
         if (isSpecialPage(destLocation) && isRelativeUrl(url))
             return;
 
-        url = ensureOriginTrailingSlash(url);
+        url = prepareUrl(url);
 
         const proxyUrl = urlUtils.getProxyUrl(url);
 

src/client/sandbox/code-instrumentation/location/wrapper.js

@@ -14,7 +14,7 @@ import {
     getResourceTypeString,
     sameOriginCheck,
     ensureTrailingSlash,
-    ensureOriginTrailingSlash
+    prepareUrl
 } from '../../../../utils/url';
 import nativeMethods from '../../native-methods';
 import urlResolver from '../../../utils/url-resolver';
@@ -62,7 +62,7 @@ export default class LocationWrapper extends EventEmitter {
             return ensureTrailingSlash(href, locationUrl);
         };
         const getProxiedHref = href => {
-            href = ensureOriginTrailingSlash(href);
+            href = prepareUrl(href);
 
             if (isJsProtocol(href))
                 return processJsAttrValue(href, { isJsProtocol: true, isEventAttr: false });

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

@@ -7,7 +7,7 @@ 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 } from '../../../../utils/url';
+import { prepareUrl } from '../../../../utils/url';
 import { cleanUpHtml, processHtml } from '../../../utils/html';
 import { getAttributesProperty } from './attributes';
 import styleProcessor from '../../../../processing/style';
@@ -266,7 +266,7 @@ export default class PropertyAccessorsInstrumentation extends SandboxBase {
                         /*eslint-disable no-restricted-properties*/
                         if (!locationWrapper) {
                             if (!isJsProtocol(location)) {
-                                const url          = ensureOriginTrailingSlash(location);
+                                const url          = prepareUrl(location);
                                 const resourceType = urlUtils.stringifyResourceType({ isIframe: true });
 
                                 owner.location = destLocation.sameOriginCheck(location.toString(), url)

src/client/sandbox/cookie/index.js

@@ -68,21 +68,22 @@ export default class CookieSandbox extends SandboxBase {
         if (parsedCookie.httponly)
             return false;
 
+        const parsedDestLocation = destLocation.getParsed();
+
         /*eslint-disable no-restricted-properties*/
-        const destProtocol = destLocation.getParsed().protocol;
+        const destProtocol = parsedDestLocation.protocol;
         /*eslint-enable no-restricted-properties*/
 
         // NOTE: Hammerhead tunnels HTTPS requests via HTTP, so we need to validate the Secure attribute manually.
         if (parsedCookie.secure && destProtocol !== 'https:')
             return false;
 
-        // NOTE: Add a relative protocol portion to the domain, so that we can use urlUtils for the same origin check.
-        const domain = parsedCookie.domain && '//' + parsedCookie.domain;
-
 
         // NOTE: All Hammerhad sessions have the same domain, so we need to validate the Domain attribute manually
         // according to a test url.
-        return !domain || destLocation.sameOriginCheck(destLocation.get(), domain);
+        /*eslint-disable no-restricted-properties*/
+        return !parsedCookie.domain || parsedDestLocation.hostname === parsedCookie.domain;
+        /*eslint-enable no-restricted-properties*/
     }
 
     _updateClientCookieStr (cookieKey, newCookieStr) {

src/client/utils/destination-location.js

@@ -35,7 +35,7 @@ export function sameOriginCheck (location, checkedUrl, rejectForSubdomains) {
 }
 
 export function resolveUrl (url, doc) {
-    url = sharedUrlUtils.prepareUrl(url);
+    url = sharedUrlUtils.processSpecialChars(url);
 
     /*eslint-disable no-restricted-properties*/
     if (url && url.indexOf('//') === 0)

src/processing/resources/index.js

@@ -20,7 +20,7 @@ function getResourceUrlReplacer (ctx) {
 
         // NOTE: Resolves base URLs without a protocol ('//google.com/path' for example).
         baseUrl     = baseUrl ? url.resolve(ctx.dest.url, baseUrl) : '';
-        resourceUrl = urlUtil.prepareUrl(resourceUrl);
+        resourceUrl = urlUtil.processSpecialChars(resourceUrl);
 
         let resolvedUrl = url.resolve(baseUrl || ctx.dest.url, resourceUrl);
 

src/proxy/index.js

@@ -166,7 +166,7 @@ export default class Proxy extends Router {
         if (externalProxySettings)
             session.setExternalProxySettings(externalProxySettings);
 
-        url = urlUtils.ensureOriginTrailingSlash(url);
+        url = urlUtils.prepareUrl(url);
 
         return urlUtils.getProxyUrl(url, {
             proxyHostname: this.server1Info.hostname,

src/request-pipeline/context.js

@@ -5,8 +5,6 @@ import * as contentTypeUtils from '../utils/content-type';
 import genearateUniqueId from '../utils/generate-unique-id';
 
 const REDIRECT_STATUS_CODES = [301, 302, 303, 307, 308];
-const HTTP_DEFAUL_PORT      = '80';
-const HTTPS_DEFAUL_PORT     = '443';
 
 export default class RequestPipelineContext {
     constructor (req, res, serverInfo) {
@@ -51,7 +49,7 @@ export default class RequestPipelineContext {
         if (parsed) {
             const parsedResourceType = urlUtils.parseResourceType(parsed.resourceType);
 
-            let dest = {
+            const dest = {
                 url:           parsed.destUrl,
                 protocol:      parsed.destResourceInfo.protocol,
                 host:          parsed.destResourceInfo.host,
@@ -68,8 +66,6 @@ export default class RequestPipelineContext {
                 reqOrigin:     parsed.reqOrigin
             };
 
-            dest = this._omitDefaultPort(dest);
-
             return {
                 dest:      dest,
                 sessionId: parsed.sessionId
@@ -79,21 +75,6 @@ export default class RequestPipelineContext {
         return null;
     }
 
-    _omitDefaultPort (dest) {
-        // NOTE: Browsers may send the default port in the 'referer' header. But since we compose the destination
-        // URL from it, we need to skip the port number if it's the protocol's default port. Some servers have
-        // host conditions that do not include a port number.
-        const hasDefaultPort = dest.protocol === 'https:' && dest.port === HTTPS_DEFAUL_PORT ||
-                               dest.protocol === 'http:' && dest.port === HTTP_DEFAUL_PORT;
-
-        if (hasDefaultPort) {
-            dest.host = dest.host.split(':')[0];
-            dest.port = '';
-        }
-
-        return dest;
-    }
-
     _getDestFromReferer (parsedReferer) {
         const dest = parsedReferer.dest;
 

src/request-pipeline/header-transforms.js

@@ -49,7 +49,7 @@ function transformCookie (src, ctx) {
 }
 
 function resolveAndGetProxyUrl (url, ctx) {
-    url = urlUtils.ensureOriginTrailingSlash(url);
+    url = urlUtils.prepareUrl(url);
 
     const { host }    = parseUrl(url);
     let isCrossDomain = false;

src/session/cookies.js

@@ -1,19 +1,39 @@
-import { CookieJar } from 'tough-cookie';
+import { CookieJar, Cookie } from 'tough-cookie';
 import BYTES_PER_COOKIE_LIMIT from './cookie-limit';
 import { castArray } from 'lodash';
+import { parseUrl } from '../utils/url';
+
+const LOCALHOST_DOMAIN = 'localhost';
+const LOCALHOST_IP     = '127.0.0.1';
 
 export default class Cookies {
     constructor () {
         this.cookieJar = new CookieJar();
     }
 
+    static _hasLocalhostDomain (cookie) {
+        if (cookie)
+            return cookie.domain === LOCALHOST_DOMAIN || cookie.domain === LOCALHOST_IP;
+
+        return false;
+    }
+
     _set (url, cookies, isClient) {
         cookies = castArray(cookies);
 
         cookies.forEach(cookieStr => {
             if (cookieStr.length > BYTES_PER_COOKIE_LIMIT)
                 return;
 
+            const cookie = Cookie.parse(cookieStr);
+
+            // NOTE: If cookie.domain and url hostname are equal to localhost/127.0.0.1,
+            // we should remove 'Domain=...' form cookieStr (GH-1491)
+            if (Cookies._hasLocalhostDomain(cookie) && parseUrl(url).hostname === cookie.domain) {
+                cookie.domain = '';
+                cookieStr     = cookie.toString();
+            }
+
             this.cookieJar.setCookieSync(cookieStr, url, {
                 http:        !isClient,
                 ignoreError: true,

src/utils/url.js

@@ -20,6 +20,9 @@ export const REQUEST_DESCRIPTOR_VALUES_SEPARATOR = '!';
 export const TRAILING_SLASH_RE                   = /\/$/;
 export const SPECIAL_PAGES                       = ['about:blank', 'about:error'];
 
+export const HTTP_DEFAULT_PORT  = '80';
+export const HTTPS_DEFAULT_PORT = '443';
+
 export function parseResourceType (resourceType) {
     if (!resourceType) {
         return {
@@ -218,7 +221,7 @@ export function getPathname (path) {
 export function parseUrl (url) {
     const parsed = {};
 
-    url = prepareUrl(url);
+    url = processSpecialChars(url);
 
     if (!url)
         return parsed;
@@ -247,6 +250,7 @@ export function parseUrl (url) {
     parsed.partAfterHost = remainder
         .replace(HOST_RE, (str, host, restPartSeparator) => {
             parsed.host = host;
+            parsed.port = '';
             return restPartSeparator;
         });
 
@@ -315,7 +319,7 @@ export function formatUrl (parsedUrl) {
     return url;
 }
 
-export function prepareUrl (url) {
+export function processSpecialChars (url) {
     // TODO: fix it
     /* eslint-disable no-undef */
     if (url === null && /iPad|iPhone/i.test(window.navigator.userAgent))
@@ -384,3 +388,28 @@ export function ensureOriginTrailingSlash (url) {
 
     return url;
 }
+
+export function omitDefaultPort (url) {
+    // NOTE: If you request an url containing default port
+    // then browser remove this one itself.
+    const parsedUrl = parseUrl(url);
+
+    const hasDefaultPort = parsedUrl.protocol === 'https:' && parsedUrl.port === HTTPS_DEFAULT_PORT ||
+                           parsedUrl.protocol === 'http:' && parsedUrl.port === HTTP_DEFAULT_PORT;
+
+    if (hasDefaultPort) {
+        parsedUrl.host = parsedUrl.hostname;
+        parsedUrl.port = '';
+
+        return formatUrl(parsedUrl);
+    }
+
+    return url;
+}
+
+export function prepareUrl (url) {
+    url = omitDefaultPort(url);
+    url = ensureOriginTrailingSlash(url);
+
+    return url;
+}

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

@@ -264,6 +264,72 @@ test('should ensure a trailing slash on page navigation using hammerhead.navigat
     hammerhead.win = storedWindow;
 });
 
+test('should omit the default port on page navigation', function () {
+    var storedWindow = hammerhead.win;
+
+    var PORT_RE = /:([0-9][0-9]*)/;
+
+    function getExpectedProxyUrl (url, shouldOmitPort) {
+        var expectedUrl = shouldOmitPort ? url.replace(PORT_RE, '') : url;
+
+        return urlUtils.getProxyUrl(expectedUrl);
+    }
+
+    var windowMock = {
+        location: {
+            href: '',
+
+            replace: function (url) {
+                this.href = url;
+            },
+
+            assign: function (url) {
+                this.href = url;
+            },
+
+            toString: function () {
+                return urlUtils.getProxyUrl(this.location.href);
+            }
+        }
+    };
+
+    windowMock.top = windowMock;
+
+    var locationWrapper = new LocationWrapper(windowMock);
+
+    hammerhead.win = {
+        location: ''
+    };
+
+    function testUrl (url, shouldOmitPort) {
+        locationWrapper.href = url;
+        strictEqual(windowMock.location.href, getExpectedProxyUrl(url, shouldOmitPort), 'href = ' + url);
+
+        locationWrapper.replace(url);
+        strictEqual(windowMock.location.href, getExpectedProxyUrl(url, shouldOmitPort), 'replace(' + url + ')');
+
+        locationWrapper.assign(url);
+        strictEqual(windowMock.location.href, getExpectedProxyUrl(url, shouldOmitPort), 'assign(' + url + ')');
+
+        hammerhead.navigateTo(url);
+        strictEqual(hammerhead.win.location, getExpectedProxyUrl(url, shouldOmitPort), 'navigateTo(' + url + ')');
+    }
+
+    function testDefaultPortOmitting (protocol, defaultPort, defaultPortForAnotherProtocol) {
+        testUrl(protocol + '//localhost:' + defaultPort + '/', true);
+        testUrl(protocol + '//127.0.0.1:' + defaultPort + '/', true);
+        testUrl(protocol + '//example.com:' + defaultPort + '/', true);
+        testUrl(protocol + '//example.com:' + defaultPort + '/page.html', true);
+        testUrl(protocol + '//localhost:' + defaultPortForAnotherProtocol + '/', false);
+        testUrl(protocol + '//localhost:2343/', false);
+    }
+
+    testDefaultPortOmitting('http:', '80', '443');
+    testDefaultPortOmitting('https:', '443', '80');
+
+    hammerhead.win = storedWindow;
+});
+
 module('regression');
 
 if (browserUtils.compareVersions([browserUtils.webkitVersion, '603.1.30']) === -1) {