Skip to content

Commit

Permalink
Backport PR #6773
Browse files Browse the repository at this point in the history
---------

**Commit 1:**
[state] add configurable warning level based on url length

* Original sha: 55db90d
* Authored by spalger <[email protected]> on 2016-04-04T21:57:45Z

**Commit 2:**
[state] add a hard length limit that will start throwing errors

* Original sha: 64699aa
* Authored by spalger <[email protected]> on 2016-04-04T22:14:06Z

**Commit 3:**
Merge branch 'master' of github.com:elastic/kibana into implement/betterWarningForUrlLength

* Original sha: 4c70d69
* Authored by spalger <[email protected]> on 2016-04-06T07:05:47Z

**Commit 4:**
[dashboard] cleanup quietly to prevent error

* Original sha: 1dace5c
* Authored by spalger <[email protected]> on 2016-04-06T08:50:55Z

**Commit 5:**
[errorview] add url overflow display

* Original sha: 8b4ebf5
* Authored by spalger <[email protected]> on 2016-04-06T08:51:47Z

**Commit 6:**
[errorview] persist the overflow url so that refresh works

* Original sha: e308db9
* Authored by spalger <[email protected]> on 2016-04-06T09:05:13Z

**Commit 7:**
Merge branch 'master' of github.com:elastic/kibana into implement/betterWarningForUrlLength

* Original sha: 121e4f2
* Authored by spalger <[email protected]> on 2016-04-15T18:49:12Z

**Commit 8:**
[config] remove url limit config, it should adapt automatically

* Original sha: 281b38b
* Authored by spalger <[email protected]> on 2016-04-15T23:38:05Z

**Commit 9:**
[chrome] rework url overflow detection

The previous version of this pr relied on the State service to catch times when the URL would grow out of control. While overflows will commonly occur in the State service, this didn't handle urls that were navigated to using a link. They worked because the state service would eventually be called, but the failure was unexpected and required interaction to trigger.

This new approach does the checking at a higher level, in the chrome.

We also removed the `url:limit` configuration value in favor of browser detection (I was not able to find or come up with a way to quietly and quickly feature detect this). The new limits are 2000 characters for IE and 25000 for all other browsers.

* Original sha: 116521c
* Authored by spalger <[email protected]> on 2016-04-15T23:40:18Z

**Commit 10:**
Merge branch 'master' of github.com:elastic/kibana into implement/betterWarningForUrlLength

* Original sha: aa030d7
* Authored by spalger <[email protected]> on 2016-04-26T16:51:22Z

**Commit 11:**
[ui/config] remove unused default

* Original sha: b333c51
* Authored by spalger <[email protected]> on 2016-04-26T16:52:29Z

**Commit 12:**
Merge branch 'master' of github.com:elastic/kibana into implement/betterWarningForUrlLength

* Original sha: 133d7e5
* Authored by spalger <[email protected]> on 2016-04-27T23:17:44Z

**Commit 13:**
[urlOverflow] assign magic numbers to variables

* Original sha: 8b97881
* Authored by spalger <[email protected]> on 2016-04-27T23:24:17Z
  • Loading branch information
epixa committed Aug 15, 2016
1 parent 2868233 commit 89bbb05
Show file tree
Hide file tree
Showing 8 changed files with 160 additions and 7 deletions.
7 changes: 4 additions & 3 deletions src/plugins/kibana/public/dashboard/directives/grid.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,15 @@ define(function (require) {
});

$scope.$on('$destroy', function () {
safeLayout.cancel();
$window.off('resize', safeLayout);

if (!gridster) return;
gridster.$widgets.each(function (i, el) {
const panel = getPanelFor(el);
removePanel(panel);
// stop any animations
panel.$el.stop();
removePanel(panel, true);
// not that we will, but lets be safe
makePanelSerializeable(panel);
});
Expand Down Expand Up @@ -125,9 +126,9 @@ define(function (require) {
}

// tell gridster to remove the panel, and cleanup our metadata
function removePanel(panel) {
function removePanel(panel, silent) {
// remove from grister 'silently' (don't reorganize after)
gridster.remove_widget(panel.$el);
gridster.remove_widget(panel.$el, silent);

// destroy the scope
panel.$scope.$destroy();
Expand Down
32 changes: 31 additions & 1 deletion src/ui/public/chrome/api/angular.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
let _ = require('lodash');
import { format as formatUrl, parse as parseUrl } from 'url';

import Notifier from 'ui/notify/notifier';
import { UrlOverflowServiceProvider } from '../../error_url_overflow';

const URL_LIMIT_WARN_WITHIN = 150;

module.exports = function (chrome, internals) {

Expand All @@ -20,7 +26,31 @@ module.exports = function (chrome, internals) {
a.href = chrome.addBasePath('/elasticsearch');
return a.href;
}()))
.config(chrome.$setupXsrfRequestInterceptor);
.config(chrome.$setupXsrfRequestInterceptor)
.run(($location, $rootScope, Private) => {
const notify = new Notifier();
const urlOverflow = Private(UrlOverflowServiceProvider);
const check = (event) => {
if ($location.path() === '/error/url-overflow') return;

try {
if (urlOverflow.check($location.absUrl()) <= URL_LIMIT_WARN_WITHIN) {
notify.warning(`
The URL has gotten big and may cause Kibana
to stop working. Please simplify the data on screen.
`);
}
} catch (e) {
const { host, path, search, protocol } = parseUrl(window.location.href);
// rewrite the entire url to force the browser to reload and
// discard any potentially unstable state from before
window.location.href = formatUrl({ host, path, search, protocol, hash: '#/error/url-overflow' });
}
};

$rootScope.$on('$routeUpdate', check);
$rootScope.$on('$routeChangeStart', check);
});

require('../directives')(chrome, internals);

Expand Down
2 changes: 1 addition & 1 deletion src/ui/public/config/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ define(function (require) {
'dashboard:defaultDarkTheme': {
value: false,
description: 'New dashboards use dark theme by default',
}
},
};
};
});
18 changes: 18 additions & 0 deletions src/ui/public/error_url_overflow/error_url_overflow.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<div class="app-container error-url-overflow-app">
<h3>
<i aria-hidden="true" class="fa fa-warning text-danger"></i> Woah there!
</h3>
<p>
That's a big URL you have there. I have some unfortunate news: Your browser doesn't play nice with Kibana's bacon-double-cheese-burger-with-extra-fries sized URL. To keep you from running into problems Kibana limits URLs in your browser to {{controller.limit}} characters for your safety.
</p>

<h3>Ok, how do I fix this?</h3>
<p>This usually only happens with big, complex dashboards, so you have some options:</p>
<ol>
<li>Remove some stuff from your dashboard. This will reduce the length of the URL and keep IE in a good place.</li>
<li>Don't use IE. Every other supported browser we know of doesn't have this limit.</li>
</ol>
<br>
<br>
<p><small>Foot note: Party size candy bars are tiny. Party size sub sandwiches are massive. Really makes you think.</small></p>
</div>
30 changes: 30 additions & 0 deletions src/ui/public/error_url_overflow/error_url_overflow.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import uiRoutes from 'ui/routes';
import uiModules from 'ui/modules';
import KbnUrlProvider from 'ui/url';

import './error_url_overflow.less';
import template from './error_url_overflow.html';
import { UrlOverflowServiceProvider } from './url_overflow_service';

export * from './url_overflow_service';

uiRoutes
.when('/error/url-overflow', {
template,
controllerAs: 'controller',
controller: class OverflowController {
constructor(Private, config, $scope) {
const kbnUrl = Private(KbnUrlProvider);
const urlOverflow = Private(UrlOverflowServiceProvider);

if (!urlOverflow.get()) {
kbnUrl.redirectPath('/');
return;
}

this.url = urlOverflow.get();
this.limit = urlOverflow.failLength();
$scope.$on('$destroy', () => urlOverflow.clear());
}
}
});
7 changes: 7 additions & 0 deletions src/ui/public/error_url_overflow/error_url_overflow.less
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
.error-url-overflow-app {
padding: 25px;

pre {
white-space: pre-wrap;
}
}
65 changes: 65 additions & 0 deletions src/ui/public/error_url_overflow/url_overflow_service.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
const URL_MAX_IE = 2000;
const URL_MAX_OTHERS = 25000;
const IE_REGEX = /(;MSIE |Edge\/\d)/;

export class UrlOverflowService {
constructor() {
const key = 'error/url-overflow/url';
const store = window.sessionStorage || {
getItem() {},
setItem() {},
removeItem() {},
};

// FIXME: Couldn't find a way to test for browser compatibility without
// complex redirect and cookie based "feature-detection" page, so going
// with user-agent detection for now.
this._ieLike = IE_REGEX.test(window.navigator.userAgent);

this._val = store.getItem(key);
this._sync = () => {
if (this._val == null) store.removeItem(key);
else store.setItem(key, this._val);
};
}

failLength() {
return this._ieLike ? URL_MAX_IE : URL_MAX_OTHERS;
}

set(v) {
this._val = v;
this._sync();
}

get() {
return this._val;
}

check(absUrl) {
if (!this.get()) {
const urlLength = absUrl.length;
const remaining = this.failLength() - urlLength;

if (remaining > 0) {
return remaining;
}

this.set(absUrl);
}

throw new Error(`
The URL has gotten too big and kibana can no longer
continue. Please refresh to return to your previous state.
`);
}

clear() {
this._val = undefined;
this._sync();
}
}

export function UrlOverflowServiceProvider() {
return new UrlOverflowService();
}
6 changes: 4 additions & 2 deletions src/ui/public/state_management/state.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import KbnUrlProvider from 'ui/url';

define(function (require) {
let _ = require('lodash');
let rison = require('ui/utils/rison');
Expand All @@ -6,7 +8,8 @@ define(function (require) {
let qs = require('ui/utils/query_string');

return function StateProvider(Notifier, Private, $rootScope, $location) {
let Events = Private(require('ui/events'));
const notify = new Notifier();
const Events = Private(require('ui/events'));

_.class(State).inherits(Events);
function State(urlParam, defaults) {
Expand Down Expand Up @@ -43,7 +46,6 @@ define(function (require) {
try {
return search[this._urlParam] ? rison.decode(search[this._urlParam]) : null;
} catch (e) {
let notify = new Notifier();
notify.error('Unable to parse URL');
search[this._urlParam] = rison.encode(this._defaults);
$location.search(search).replace();
Expand Down

0 comments on commit 89bbb05

Please sign in to comment.