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

[Discover] View single document does not URL encode id #158525

Closed
pickypg opened this issue May 25, 2023 · 2 comments · Fixed by #158635
Closed

[Discover] View single document does not URL encode id #158525

pickypg opened this issue May 25, 2023 · 2 comments · Fixed by #158635
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Discover Discover Application impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) v8.8.1
Projects

Comments

@pickypg
Copy link
Member

pickypg commented May 25, 2023

Kibana version: 8.7.0

Describe the bug:

When selecting "View single document" in Discover, it is expected to go to a page with just that document. However, if the ID has characters that require escaping, then it fails to load because the query string id is not URL encoded.

In the breadcrumb from the screenshot below, the ID has been URL decoded, but it was never encoded.

Steps to reproduce:

  1. Create a document with a +(or another character that needs to be escaped) in its _id
  2. Select it from Discover to View single document
  3. Observe the same failure

Expected behavior:

It should URL encode the _id in the id query string variable.

Screenshots (if relevant):

image
@pickypg pickypg added the bug Fixes for quality problems that affect the customer experience label May 25, 2023
@botelastic botelastic bot added the needs-team Issues missing a team label label May 25, 2023
@pickypg pickypg changed the title [Discover] Show [Discover] View single document does not URL encode id May 25, 2023
@dej611 dej611 added Feature:Discover Discover Application Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) labels May 26, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@botelastic botelastic bot removed the needs-team Issues missing a team label label May 26, 2023
@kibanamachine kibanamachine added this to Inbox in Discover May 26, 2023
@davismcphee davismcphee added v8.7.0 loe:small Small Level of Effort impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. labels May 29, 2023
@davismcphee davismcphee self-assigned this May 29, 2023
@davismcphee
Copy link
Contributor

@pickypg Thanks for reporting! I opened a PR with a fix here: #158635. It'll only land in 8.8.1+ though since all 8.7.x releases have shipped already.

davismcphee added a commit that referenced this issue May 30, 2023
)

## Summary

Fixes #158525.

### Checklist

- [ ] ~Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~
- [ ]
~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials~
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] ~Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard
accessibility](https://webaim.org/techniques/keyboard/))~
- [ ] ~Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~
- [ ] ~If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~
- [ ] ~This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~
- [ ] ~This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)~

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue May 30, 2023
…tic#158635)

## Summary

Fixes elastic#158525.

### Checklist

- [ ] ~Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~
- [ ]
~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials~
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] ~Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard
accessibility](https://webaim.org/techniques/keyboard/))~
- [ ] ~Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~
- [ ] ~If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~
- [ ] ~This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~
- [ ] ~This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)~

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 097666f)
kibanamachine added a commit that referenced this issue May 30, 2023
…#158635) (#158684)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Discover] Update single doc view locator to URL encode `rowId`
(#158635)](#158635)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Davis
McPhee","email":"davis.mcphee@elastic.co"},"sourceCommit":{"committedDate":"2023-05-30T21:57:45Z","message":"[Discover]
Update single doc view locator to URL encode `rowId` (#158635)\n\n##
Summary\r\n\r\nFixes #158525.\r\n\r\n### Checklist\r\n\r\n- [ ] ~Any
text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~\r\n-
[
]\r\n~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials~\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] ~Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard\r\naccessibility](https://webaim.org/techniques/keyboard/))~\r\n-
[ ] ~Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~\r\n-
[ ] ~If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~\r\n-
[ ] ~This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~\r\n-
[ ] ~This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)~\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"097666f1a0f4394d68fb0f2a8e7581d5898d0a97","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Discover","release_note:fix","Team:DataDiscovery","v8.9.0","v8.8.1"],"number":158635,"url":"https://github.com/elastic/kibana/pull/158635","mergeCommit":{"message":"[Discover]
Update single doc view locator to URL encode `rowId` (#158635)\n\n##
Summary\r\n\r\nFixes #158525.\r\n\r\n### Checklist\r\n\r\n- [ ] ~Any
text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~\r\n-
[
]\r\n~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials~\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] ~Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard\r\naccessibility](https://webaim.org/techniques/keyboard/))~\r\n-
[ ] ~Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~\r\n-
[ ] ~If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~\r\n-
[ ] ~This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~\r\n-
[ ] ~This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)~\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"097666f1a0f4394d68fb0f2a8e7581d5898d0a97"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/158635","number":158635,"mergeCommit":{"message":"[Discover]
Update single doc view locator to URL encode `rowId` (#158635)\n\n##
Summary\r\n\r\nFixes #158525.\r\n\r\n### Checklist\r\n\r\n- [ ] ~Any
text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~\r\n-
[
]\r\n~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials~\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] ~Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard\r\naccessibility](https://webaim.org/techniques/keyboard/))~\r\n-
[ ] ~Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~\r\n-
[ ] ~If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~\r\n-
[ ] ~This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~\r\n-
[ ] ~This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)~\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"097666f1a0f4394d68fb0f2a8e7581d5898d0a97"}},{"branch":"8.8","label":"v8.8.1","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Davis McPhee <davis.mcphee@elastic.co>
@kertal kertal added v8.8.1 and removed v8.7.0 labels Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Discover Discover Application impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) v8.8.1
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants