[Maps] prevent users from overflowing URL when filtering by shape #50747
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nreese
added
release_note:fix
[Deprecated-Use Team:Presentation]Team:Geo
|
Pinging @elastic/kibana-gis (Team:Geo) |
|
@gchaps added you as a review for the error message displayed when the filter can not be created. |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
|
retest |
💚 Build Succeeded |
gchaps
reviewed
Nov 15, 2019
thomasneirynck
approved these changes
Nov 18, 2019
...lugins/maps/public/connected_components/map/features_tooltip/feature_geometry_filter_form.js
Show resolved
Hide resolved
...lugins/maps/public/connected_components/map/features_tooltip/feature_geometry_filter_form.js
Outdated
Show resolved
Hide resolved
@gchaps The more I think about this message, I think it needs to mention the real limitation which is overflowing the URL. How about something like "Cannot create filter because filter will overflow URL." |
|
@nreese I'm not sure what you mean by "overflow the URL means". Can you be more specific? |
💚 Build Succeeded |
|
Chatted with @gchaps and came up with "Cannot create filter. Filters are added to the URL, and this shape has too many vertices to fit in the URL." |
💚 Build Succeeded |
nreese
added a commit
to nreese/kibana
that referenced
this pull request
Nov 18, 2019
…astic#50747) * [Maps] prevent users from overflowing URL when filtering by shape * small fix * fix jest test * update warning message * update overflow error message
nreese
added a commit
to nreese/kibana
that referenced
this pull request
Nov 18, 2019
…astic#50747) * [Maps] prevent users from overflowing URL when filtering by shape * small fix * fix jest test * update warning message * update overflow error message
💚 Build Succeeded |
jloleysens
added a commit
to jloleysens/kibana
that referenced
this pull request
Nov 18, 2019
…-fallback * 'master' of github.com:elastic/kibana: (116 commits) [Maps] move apply global filter settting from layer to source (elastic#50523) [SIEM] Fix: Empty `Source` / `Destination` shown when only ports are populated (elastic#50843) [Maps] Delay vector tile layer syncing until spritesheet is loaded (elastic#48955) [Maps] prevent users from overflowing URL when filtering by shape (elastic#50747) [DOCS] Mark Beats central management as discontinued (elastic#49423) [page_objects/common_page] convert to ts (elastic#50771) [NP Kibana Migrations ] kibana plugin home (elastic#50444) [DOCS] Shareables naming convention (elastic#50497) [ML] DF Analytics - auto-populate model_memory_limit (elastic#50714) Increase alerting test stability and reduce flakiness (elastic#50246) [ML] Remaning new_job_new folder (elastic#50917) [Telemetry] Show opt-in changes for OSS users (elastic#50831) [ML] Fix lat_long anomalies table links menu and value formatting (elastic#50916) [Dev] Fix serialising a really big string (elastic#50915) Better explanation about the Prettier recommendation (extension vs. NPM module) (elastic#50629) [Monitoring] Use a basic monitoring user for tests (elastic#47865) [Monitoring] Gracefully handle issue with filebeat indices (elastic#48929) [Monitoring] Improve permissions required around setup mode (elastic#50421) Additional validation for elasticsearch username (elastic#48247) Revert changes to use_kibana_ui_setting (elastic#50877) ... # Conflicts: # src/legacy/core_plugins/console/server/request.test.ts
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #50173 and #50652.
related to #45521
Creating a filter by shape can overflow the URL since the filter goes into app state which is part of the URL.
Once a filter that overflows the URL, #50173 was exposed. There is a bug in angular_config method $setupUrlOverflowHandling at https://github.com/elastic/kibana/blob/master/src/legacy/ui/public/legacy_compat/angular_config.tsx#L356.
modifyUrldoes not exist. PR #38237 removedsrc/core/public/utils/modify_url.tswhich was duplicated insrc/core/utils/url.tsbut failed to update the path referenced byui/url/index.js. Since the path referenced byui/url/index.jsstill hit an index.js file, no build exception occurred.This PR prevents users from overflowing the URL by not allowing them to create filters that are super long. Not the best solution but better then the havoc found in issue #50652.
This is pretty limiting since pre-indexed shapes can not be used to query geo_point indices yet, but given the problems URL overflows are causing, this PR is better than the alternative.
To test, create a filter from the United States EMS boundary that can be found in the ecommerce sample data or web logs sample data.
cc @bhavyarm you will need to remove the session state
error/url-overflow/urlto clean up your browser before everything will work correctly.