fix 'Page without slash after location's host reloads if we set only …

…hash' (close #1426) (#1514)

* i1426

* i1426

* Requested changes: rename ensureHostEndedTrailingSlash() to ensureOriginTrailingSlash(); rewrite urlUtils.getProxyUrl(ensureOriginTrailingSlash(url))

* Fix server test (Should procees "x-frame-options" header (GH-1017)), export TRAILING_SLASH_RE needed in this test, fix client location test

* Fix navigateTo(): leading slashes case

* Fix ensureOriginTrailingSlash(), code cleanup

* Add ensureOriginTrailingSlash test

* Fix location header, fix ensureOriginTrailingSlash test, add location header test, add location href test (wrapper)

* Add tests

* Requested changes

* Requested changes

* 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 } from '../utils/url';
+import { isRelativeUrl, parseProxyUrl, isSpecialPage, ensureOriginTrailingSlash } from '../utils/url';
 import * as urlUtils from './utils/url';
 import * as featureDetection from './utils/feature-detection';
 import * as htmlUtils from './utils/html';
@@ -197,10 +197,14 @@ class Hammerhead {
             destLocation = parsedProxyUrl && parsedProxyUrl.destUrl;
         }
 
-        if (destLocation === 'about:blank' && isRelativeUrl(url))
+        if (isSpecialPage(destLocation) && isRelativeUrl(url))
             return;
 
-        this.win.location = urlUtils.getProxyUrl(url);
+        url = ensureOriginTrailingSlash(url);
+
+        const proxyUrl = urlUtils.getProxyUrl(url);
+
+        this.win.location = proxyUrl;
     }
 
     start (initSettings, win) {

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

@@ -9,7 +9,13 @@ import {
     isChangedOnlyHash,
     getCrossDomainProxyPort
 } from '../../../utils/url';
-import { getDomain, getResourceTypeString, sameOriginCheck, ensureTrailingSlash } from '../../../../utils/url';
+import {
+    getDomain,
+    getResourceTypeString,
+    sameOriginCheck,
+    ensureTrailingSlash,
+    ensureOriginTrailingSlash
+} from '../../../../utils/url';
 import nativeMethods from '../../native-methods';
 import urlResolver from '../../../utils/url-resolver';
 import { processJsAttrValue, isJsProtocol } from '../../../../processing/dom/index';
@@ -56,6 +62,8 @@ export default class LocationWrapper extends EventEmitter {
             return ensureTrailingSlash(href, locationUrl);
         };
         const getProxiedHref = href => {
+            href = ensureOriginTrailingSlash(href);
+
             if (isJsProtocol(href))
                 return processJsAttrValue(href, { isJsProtocol: true, isEventAttr: false });
 

src/proxy/index.js

@@ -166,6 +166,8 @@ export default class Proxy extends Router {
         if (externalProxyUrl)
             session.setExternalProxySettings(externalProxyUrl);
 
+        url = urlUtils.ensureOriginTrailingSlash(url);
+
         return urlUtils.getProxyUrl(url, {
             proxyHostname: this.server1Info.hostname,
             proxyPort:     this.server1Info.port,

src/request-pipeline/header-transforms.js

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

src/utils/url.js

@@ -12,11 +12,12 @@ const HOST_RE            = /^(.*?)(\/|%|\?|;|#|$)/;
 const PORT_RE            = /:([0-9]*)$/;
 const QUERY_AND_HASH_RE  = /(\?.+|#[^#]*)$/;
 const PATH_AFTER_HOST_RE = /^\/([^/]+?)\/([\S\s]+)$/;
-const TRAILING_SLASH_RE  = /\/$/;
+const HTTP_RE            = /^(?:https?):/;
 
 export const SUPPORTED_PROTOCOL_RE               = /^(?:https?|file):/i;
 export const HASH_RE                             = /^#/;
 export const REQUEST_DESCRIPTOR_VALUES_SEPARATOR = '!';
+export const TRAILING_SLASH_RE                   = /\/$/;
 export const SPECIAL_PAGES                       = ['about:blank', 'about:error'];
 
 export function parseResourceType (resourceType) {
@@ -369,3 +370,14 @@ export function isValidUrl (url) {
 
     return parsedUrl.protocol === 'file:' || parsedUrl.hostname && (!parsedUrl.port || isValidPort(parsedUrl.port));
 }
+
+export function ensureOriginTrailingSlash (url) {
+    // NOTE: If you request an url containing only port, host and protocol
+    // then browser adds the trailing slash itself.
+    const parsedUrl = parseUrl(url);
+
+    if (!parsedUrl.partAfterHost && HTTP_RE.test(parsedUrl.protocol))
+        return url + '/';
+
+    return url;
+}

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

@@ -11,6 +11,41 @@ var nativeMethods = hammerhead.nativeMethods;
 var browserUtils  = hammerhead.utils.browser;
 var domUtils      = hammerhead.utils.dom;
 
+var ENSURE_URL_TRAILING_SLASH_TEST_CASES = [
+    {
+        url:                   'http://example.com',
+        shoudAddTrailingSlash: true
+    },
+    {
+        url:                   'https://localhost:8080',
+        shoudAddTrailingSlash: true
+    },
+    {
+        url:                   'about:blank',
+        shoudAddTrailingSlash: false
+    },
+    {
+        url:                   'http://example.com/page.html',
+        shoudAddTrailingSlash: false
+    },
+    {
+        url:                   'file://localhost/etc/fstab', // Unix file URI scheme
+        shoudAddTrailingSlash: false
+    },
+    {
+        url:                   'file:///etc/fstab', // Unix file URI scheme
+        shoudAddTrailingSlash: false
+    },
+    {
+        url:                   'file://localhost/c:/WINDOWS/clock.avi', // Windows file URI scheme
+        shoudAddTrailingSlash: false
+    },
+    {
+        url:                   'file:///c:/WINDOWS/clock.avi', // Windows file URI scheme
+        shoudAddTrailingSlash: false
+    }
+];
+
 test('iframe with empty src', function () {
     function assert (iframe) {
         new CodeInstrumentation({}, {}).attach(iframe.contentWindow);
@@ -112,7 +147,7 @@ test('iframe', function () {
     wrapper = LocationInstrumentation.getLocationWrapper(windowMock);
 
     wrapper.assign('http://new.domain.com:1444');
-    strictEqual(windowMock.location.toString(), getProxy('http://new.domain.com:1444'));
+    strictEqual(windowMock.location.toString(), getProxy('http://new.domain.com:1444/'));
 
     wrapper.replace('https://domain.com:1555/index.html');
     strictEqual(windowMock.location.toString(), getProxy('https://domain.com:1555/index.html'));
@@ -157,6 +192,78 @@ test('special pages (GH-339)', function () {
     destLocation.forceLocation(storedForcedLocation);
 });
 
+test('should ensure a trailing slash on page navigation using href setter, assign and replace methods (GH-1426)', function () {
+    function getExpectedProxyUrl (testCase) {
+        var proxiedUrl = urlUtils.getProxyUrl(testCase.url);
+
+        return proxiedUrl + (testCase.shoudAddTrailingSlash ? '/' : '');
+    }
+
+    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);
+
+    function testAddingTrailingSlash (testCases) {
+        testCases.forEach(function (testCase) {
+            locationWrapper.href = testCase.url;
+            strictEqual(windowMock.location.href, getExpectedProxyUrl(testCase));
+
+            locationWrapper.replace(testCase.url);
+            strictEqual(windowMock.location.href, getExpectedProxyUrl(testCase));
+
+            locationWrapper.assign(testCase.url);
+            strictEqual(windowMock.location.href, getExpectedProxyUrl(testCase));
+        });
+    }
+
+    testAddingTrailingSlash(ENSURE_URL_TRAILING_SLASH_TEST_CASES);
+});
+
+test('should ensure a trailing slash on page navigation using hammerhead.navigateTo method (GH-1426)', function () {
+    var storedWindow = hammerhead.win;
+
+    function getExpectedProxyUrl (testCase) {
+        var proxiedUrl = urlUtils.getProxyUrl(testCase.url);
+
+        return proxiedUrl + (testCase.shoudAddTrailingSlash ? '/' : '');
+    }
+
+    var windowMock = {
+        location: ''
+    };
+
+    hammerhead.win = windowMock;
+
+    function testAddingTrailingSlash (testCases) {
+        testCases.forEach(function (testCase) {
+            hammerhead.navigateTo(testCase.url);
+            strictEqual(hammerhead.win.location, getExpectedProxyUrl(testCase));
+        });
+    }
+
+    testAddingTrailingSlash(ENSURE_URL_TRAILING_SLASH_TEST_CASES);
+
+    hammerhead.win = storedWindow;
+});
+
 module('regression');
 
 if (browserUtils.compareVersions([browserUtils.webkitVersion, '603.1.30']) === -1) {
@@ -280,7 +387,7 @@ test('emulate a native browser behaviour related with trailing slashes for locat
     var overrideGetResolverElement = function (resolvedHref) {
         urlResolver.getResolverElement = function () {
             var storedAnchorHrefGetter = nativeMethods.anchorHrefGetter;
-            var anchor = document.createElement('a');
+            var anchor                 = document.createElement('a');
 
             nativeMethods.anchorHrefGetter = function () {
                 nativeMethods.anchorHrefGetter = storedAnchorHrefGetter;

test/server/proxy-test.js

@@ -29,6 +29,41 @@ const nodeVersion                          = parseFloat(require('node-version').
 
 const EMPTY_PAGE_MARKUP = '<html></html>';
 
+const ENSURE_URL_TRAILING_SLASH_TEST_CASES = [
+    {
+        url:                   'http://example.com',
+        shoudAddTrailingSlash: true
+    },
+    {
+        url:                   'https://localhost:8080',
+        shoudAddTrailingSlash: true
+    },
+    {
+        url:                   'about:blank',
+        shoudAddTrailingSlash: false
+    },
+    {
+        url:                   'http://example.com/page.html',
+        shoudAddTrailingSlash: false
+    },
+    {
+        url:                   'file://localhost/etc/fstab', // Unix file URI scheme
+        shoudAddTrailingSlash: false
+    },
+    {
+        url:                   'file:///etc/fstab', // Unix file URI scheme
+        shoudAddTrailingSlash: false
+    },
+    {
+        url:                   'file://localhost/c:/WINDOWS/clock.avi', // Windows file URI scheme
+        shoudAddTrailingSlash: false
+    },
+    {
+        url:                   'file:///c:/WINDOWS/clock.avi', // Windows file URI scheme
+        shoudAddTrailingSlash: false
+    }
+];
+
 function trim (str) {
     return str.replace(/^\s+|\s+$/g, '');
 }
@@ -358,9 +393,31 @@ describe('Proxy', () => {
     });
 
     describe('Session', () => {
+        it('Should ensure a trailing slash on openSession() (GH-1426)', () => {
+            function getExpectedProxyUrl (testCase) {
+                const proxiedUrl = urlUtils.getProxyUrl(testCase.url, {
+                    proxyHostname: '127.0.0.1',
+                    proxyPort:     1836,
+                    sessionId:     session.id,
+                });
+
+                return proxiedUrl + (testCase.shoudAddTrailingSlash ? '/' : '');
+            }
+
+            function testAddingTrailingSlash (testCases) {
+                testCases.forEach(function (testCase) {
+                    const actualUrl = proxy.openSession(testCase.url, session);
+
+                    expect(actualUrl).eql(getExpectedProxyUrl(testCase));
+                });
+            }
+
+            testAddingTrailingSlash(ENSURE_URL_TRAILING_SLASH_TEST_CASES);
+        });
+
         it('Should pass DNS errors to session', done => {
             session.handlePageError = (ctx, err) => {
-                expect(err).eql('Failed to find a DNS-record for the resource at <a href="http://www.some-unresolvable.url">http://www.some-unresolvable.url</a>.');
+                expect(err).eql('Failed to find a DNS-record for the resource at <a href="http://www.some-unresolvable.url/">http://www.some-unresolvable.url/</a>.');
                 ctx.res.end();
                 done();
                 return true;
@@ -378,7 +435,7 @@ describe('Proxy', () => {
 
         it('Should pass protocol DNS errors for existing host to session', done => {
             session.handlePageError = (ctx, err) => {
-                expect(err).eql('Failed to find a DNS-record for the resource at <a href="https://127.0.0.1:2000">https://127.0.0.1:2000</a>.');
+                expect(err).eql('Failed to find a DNS-record for the resource at <a href="https://127.0.0.1:2000/">https://127.0.0.1:2000/</a>.');
                 ctx.res.end();
                 done();
                 return true;
@@ -1813,8 +1870,8 @@ describe('Proxy', () => {
                     const host = 'http://127.0.0.1:' + port;
 
                     session.handlePageError = (ctx, err) => {
-                        expect(err).eql('Failed to find a DNS-record for the resource at <a href="' + host + '">' +
-                                        host + '</a>.');
+                        expect(err).eql('Failed to find a DNS-record for the resource at <a href="' + host + '/' + '">' +
+                                        host + '/' + '</a>.');
                         ctx.res.end();
                         done();
                         return true;
@@ -2238,7 +2295,8 @@ describe('Proxy', () => {
                 },
                 {
                     url:                 proxy.openSession('http://127.0.0.1:2000/x-frame-options/ALLOW-FROM%20https%3A%2F%2Fexample.com', session),
-                    expectedHeaderValue: 'ALLOW-FROM ' + proxy.openSession('https://example.com', session)
+                    expectedHeaderValue: 'ALLOW-FROM ' +
+                                         proxy.openSession('https://example.com', session).replace(urlUtils.TRAILING_SLASH_RE, '')
                 },
                 {
                     url:                 proxy.openSession('http://127.0.0.1:2000/x-frame-options/ALLOW-FROM%20http%3A%2F%2F127.0.0.1%3A2000%2Fpage', session),
@@ -2606,5 +2664,36 @@ describe('Proxy', () => {
                 testRedirectRequestStatusCode(202, false)
             ]);
         });
+
+        it('Should ensure a trailing slash on location header (GH-1426)', () => {
+            function getExpectedProxyUrl (testCase) {
+                const proxiedUrl = urlUtils.getProxyUrl(testCase.url, {
+                    proxyHostname: '127.0.0.1',
+                    proxyPort:     1836,
+                    sessionId:     session.id
+                });
+
+                return proxiedUrl + (testCase.shoudAddTrailingSlash ? '/' : '');
+            }
+
+            const testLocationHeaderValue = testCase => {
+                const proxiedUrl = proxy.openSession(testCase.url, session);
+                const encodedUrl = encodeURIComponent(proxiedUrl);
+                const options    = {
+                    url: 'http://127.0.0.1:2000/redirect/' + encodedUrl,
+
+                    resolveWithFullResponse: true,
+                    followRedirect:          false,
+                    simple:                  false,
+                };
+
+                return request(options)
+                    .then(res => {
+                        expect(res.headers['location']).eql(getExpectedProxyUrl(testCase));
+                    });
+            };
+
+            return Promise.all(ENSURE_URL_TRAILING_SLASH_TEST_CASES.map(testLocationHeaderValue));
+        });
     });
 });