Merge pull request #556 from getsentry/fix-url-breadcrumbs

Strip protocol + host from breadcrumb URLs if they match current URL

Author: benvinegar

src/raven.js

@@ -18,6 +18,7 @@ var truncate = utils.truncate;
 var urlencode = utils.urlencode;
 var uuid4 = utils.uuid4;
 var htmlElementAsString = utils.htmlElementAsString;
+var parseUrl = utils.parseUrl;
 
 var dsnKeys = 'source protocol user pass host port path'.split(' '),
     dsnPattern = /^(?:(\w+):)?\/\/(?:(\w+)(:\w+)?@)?([\w\.-]+)(?::(\d+))?(\/.*)/;
@@ -63,7 +64,8 @@ function Raven() {
     this._breadcrumbs = [];
     this._breadcrumbLimit = 20;
     this._lastCapturedEvent = null;
-    this._lastHref = window.location && location.href;
+    this._location = window.location;
+    this._lastHref = this._location && this._location.href;
 
     for (var method in this._originalConsole) {  // eslint-disable-line guard-for-in
       this._originalConsoleMethods[method] = this._originalConsole[method];
@@ -642,6 +644,35 @@ Raven.prototype = {
         };
     },
 
+    /**
+     * Captures a breadcrumb of type "navigation", normalizing input URLs
+     * @param to the originating URL
+     * @param from the target URL
+     * @private
+     */
+    _captureUrlChange: function(from, to) {
+        var parsedLoc = parseUrl(this._location.href);
+        var parsedTo = parseUrl(to);
+        var parsedFrom = parseUrl(from);
+
+        // because onpopstate only tells you the "new" (to) value of location.href, and
+        // not the previous (from) value, we need to track the value of the current URL
+        // state ourselves
+        this._lastHref = to;
+
+        // Use only the path component of the URL if the URL matches the current
+        // document (almost all the time when using pushState)
+        if (parsedLoc.protocol === parsedTo.protocol && parsedLoc.host === parsedTo.host)
+            to = parsedTo.path;
+        if (parsedLoc.protocol === parsedFrom.protocol && parsedLoc.host === parsedFrom.host)
+            from = parsedFrom.path;
+
+        this.captureBreadcrumb('navigation', {
+            to: to,
+            from: from
+        });
+    },
+
     /**
      * Install any queued plugins
      */
@@ -795,15 +826,9 @@ Raven.prototype = {
             // TODO: remove onpopstate handler on uninstall()
             var oldOnPopState = window.onpopstate;
             window.onpopstate = function () {
-                self.captureBreadcrumb('navigation', {
-                    from: self._lastHref,
-                    to: location.href
-                });
+                var currentHref = self._location.href;
+                self._captureUrlChange(self._lastHref, currentHref);
 
-                // because onpopstate only tells you the "new" (to) value of location.href, and
-                // not the previous (from) value, we need to track the value of location.href
-                // ourselves
-                self._lastHref = location.href;
                 if (oldOnPopState) {
                     return oldOnPopState.apply(this, arguments);
                 }
@@ -814,13 +839,14 @@ Raven.prototype = {
                 // params to preserve 0 arity
                 return function(/* state, title, url */) {
                     var url = arguments.length > 2 ? arguments[2] : undefined;
-                    self.captureBreadcrumb('navigation', {
-                        to: url,
-                        from: location.href
-                    });
-                    if (url) self._lastHref = url;
+
+                    // url argument is optional
+                    if (url) {
+                        self._captureUrlChange(self._lastHref, url);
+                    }
+
                     return origPushState.apply(this, arguments);
-                }
+                };
             });
         }
 

src/utils.js

@@ -107,6 +107,18 @@ function urlencode(o) {
     return pairs.join('&');
 }
 
+// borrowed from https://tools.ietf.org/html/rfc3986#appendix-B
+// intentionally using regex and not <a/> href parsing trick because React Native and other
+// environments where DOM might not be available
+function parseUrl(url) {
+    var match = url.match(/^(([^:\/?#]+):)?(\/\/([^\/?#]*))?([^?#]*)(\?([^#]*))?(#(.*))?$/);
+    if (!match) return {};
+    return {
+        protocol: match[2],
+        host: match[4],
+        path: match[5]
+    };
+}
 function uuid4() {
     var crypto = window.crypto || window.msCrypto;
 
@@ -173,5 +185,6 @@ module.exports = {
     joinRegExp: joinRegExp,
     urlencode: urlencode,
     uuid4: uuid4,
-    htmlElementAsString: htmlElementAsString
+    htmlElementAsString: htmlElementAsString,
+    parseUrl: parseUrl
 };

test/integration/test.js

@@ -261,7 +261,7 @@ describe('integration', function () {
                   // since this is what jQuery does
                   // https://github.com/jquery/jquery/blob/master/src/ajax/xhr.js#L37
 
-                  xhr.open('GET', 'example.json')
+                  xhr.open('GET', 'example.json');
                   xhr.onreadystatechange = function () {
                       setTimeout(done);
                       // replace onreadystatechange with no-op so exception doesn't

test/raven.test.js

@@ -2088,6 +2088,26 @@ describe('Raven (public API)', function() {
         });
     });
 
+    describe('._captureUrlChange', function () {
+        it('should create a new breadcrumb from its "from" and "to" arguments', function () {
+            Raven._breadcrumbs = [];
+            Raven._captureUrlChange('/foo', '/bar');
+            assert.deepEqual(Raven._breadcrumbs, [
+                { type: 'navigation', timestamp: 0.1, data: { from: '/foo', to: '/bar' }}
+            ]);
+        });
+
+        it('should strip protocol/host if passed URLs share the same origin as location.href', function () {
+            Raven._location = { href: 'http://example.com/foo' };
+            Raven._breadcrumbs = [];
+
+            Raven._captureUrlChange('http://example.com/foo', 'http://example.com/bar');
+            assert.deepEqual(Raven._breadcrumbs, [
+                { type: 'navigation', timestamp: 0.1, data: { from: '/foo', to: '/bar' }}
+            ]);
+        });
+    });
+
     describe('.Raven.isSetup', function() {
         it('should work as advertised', function() {
             var isSetup = this.sinon.stub(Raven, 'isSetup');

test/utils.test.js

@@ -17,6 +17,7 @@ var objectMerge = utils.objectMerge;
 var truncate = utils.truncate;
 var urlencode = utils.urlencode;
 var htmlElementAsString = utils.htmlElementAsString;
+var parseUrl = utils.parseUrl;
 
 describe('utils', function () {
     describe('isUndefined', function() {
@@ -145,7 +146,35 @@ describe('utils', function () {
                 }
             }), '<img id="image-3" title="A picture of an apple" />');
         });
+    });
+
+    describe('parseUrl', function () {
+        it('should parse fully qualified URLs', function () {
+            assert.deepEqual(parseUrl('http://example.com/foo'), {
+                host: 'example.com',
+                path: '/foo',
+                protocol: 'http'
+            });
+            assert.deepEqual(parseUrl('//example.com/foo'), {
+                host: 'example.com',
+                path: '/foo',
+                protocol: undefined
+            });
+        });
 
-        it
+        it('should parse partial URLs, e.g. path only', function () {
+            assert.deepEqual(parseUrl('/foo'), {
+                host: undefined,
+                protocol: undefined,
+                path: '/foo'
+            });
+            assert.deepEqual(parseUrl('example.com/foo'), {
+                host: undefined,
+                protocol: undefined,
+                path: 'example.com/foo'
+                // this is correct! pushState({}, '', 'example.com/foo') would take you
+                // from example.com => example.com/example.com/foo (valid url).
+            });
+        });
     });
 });