Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

only check for url overflows when not hashing states #10878

Merged

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Mar 24, 2017

Fixes #10129

The url-overflow check ensures that the URL length doesn't get out of control and tries to warn users to use state hashing if the url is growing too long. Right now we still check the url length when the user has turned on state hashing, and since links to different states still use the unhashed state (so opening in a new tab still works, for instance) those urls trip the url-overflow check and cause Kibana to redirect to the url-overflow error page.

To prevent this I've added a check to the url-overflow check that ensures that state hashing is turned off before checking.

To reproduce this in master:

  1. create a massive dashboard in chrome
  2. try to open that dashboard in any IE/Edge
  3. see the error
  4. turn on state:storeInSessionStorage advanced config
  5. visit the dashboard again and see the same error

In this PR the final step will actually render the dashboard.

@spalger spalger added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc review v5.4.0 v6.0.0 labels Mar 24, 2017
@spalger spalger requested review from epixa and jbudz March 24, 2017 01:31
@jbudz
Copy link
Member

jbudz commented Mar 27, 2017

Is it expected for long urls pasted into IE/edge to throw a rison parse error instead of an overflow page in this case?

@spalger
Copy link
Contributor Author

spalger commented Mar 28, 2017

Pasting a URL will cause a risen parse error... hmm... but clicking a link from a wiki won't...

@epixa
Copy link
Contributor

epixa commented Apr 10, 2017

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v5.4.0 v6.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants