elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.76k stars 8.15k forks source link

Navigation issues with percent sign in names #82440

Open yuliacech opened 3 years ago

yuliacech commented 3 years ago

Kibana version: Current master branch

Describe the bug: Navigation doesn't work correctly for resources with names containing percent sign with other special characters or containing %25 sequence. The issue occurs in plugins:

The issue occurs because history package (react-router dependency) uses decodeURI on the url and only happens when the user reloads a page or navigates from outside of Kibana via a deep link. The issue doesn't happen when navigating inside Kibana with react-router when encodeURI is used on the url.

Explanation of match params decoding in react-router ``` const policyName = 'test%#'; const url = encodeURI(`/policies/edit/${encodeURIComponent(policyName)}`); // url is '/policies/edit/test%2525%2523' (double decoding) reactRouterNavigate(url); ``` 1. `decodeURI` is called internally in `react-router/history` package 2. address bar is set to `test%25%23` 3. match params coming from react-router is set to `test%25%23` 4. `encodeURIComponent('test%25%23')` is used in components getting match params from react-router to get the correct policyName `test%#`. When the user reloads the page, react-router decodes the value coming from the address bar: 1. `decodeURI('test%25%23')` results in match params set to `test%%23` 2. calling `encodeURIComponent('test%%23')` throws an `URIError`, so using match params directly results in the wrong policy name `test%%23`.

The issue might be fixed once react-router is updated to v6 (which uses history v5).

Related history packages issues: 505, 777, 786

Steps to reproduce:

elasticmachine commented 3 years ago

Pinging @elastic/es-ui (Team:Elasticsearch UI)

legrego commented 3 years ago

We've also encountered this on the User management screen: https://github.com/elastic/kibana/issues/83541

cee-chen commented 3 years ago

Just ran into this exactly as well with the Enterprise Search plugin. FYI that this % decoding issue does not occur for us in our standalone React app that is also on React Router 5.2.0 (although our history dependency is on 4.5.1 vs Kibana's 4.9.0 - maybe that's why?) Super odd either way - definitely ping us if we can help at all (although those RR issues look gnarly, so maybe not :)

cee-chen commented 3 years ago

Just heard back from @yakhinvadim on this - it turns out that the reason is indeed because we pinned history to 4.5.1:

We fixated history version to 4.5.1 to not use the new "Encode/decode URLs" feature that was added in history v4.6.0. That new feature was eventually removed in this PR. Maintainers decided to release it in a new major. According to roadmap they plan to release it in 5.0. At the time of writing this comment, 5.0 is still in beta, so we can't update the "history" package right now.

Not sure how debilitating this issue is for y'all, but it might be worth considering pinning history to 4.5.1 (unless there is some functionality Kibana specifically needs from 4.6+) to fix this issue.

legrego commented 2 years ago

https://github.com/elastic/kibana/issues/132600 is tracking the work to upgrade the history package to 5.0, which will hopefully resolve this issue for us.

TinaHeiligers commented 1 year ago

FYI, there's a workaround that we could investigate if the effort involved to upgrade remains blocking.

yuliacech commented 1 year ago

We encountered this issue again while adding a new index details page (see https://github.com/elastic/kibana/issues/166100). A workaround that we use is to replace the url segment with a query parameter: /indices/${indexName} -> /indices/index_details?indexName=${indexName} (https://github.com/elastic/kibana/pull/166882).

elasticmachine commented 1 week ago

Pinging @elastic/kibana-management (Team:Kibana Management)