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

Prepend relative urls #14994

Merged
merged 11 commits into from Nov 29, 2017

Conversation

Projects
None yet
4 participants
@chrisronline
Contributor

chrisronline commented Nov 16, 2017

Fixes ESA-2017-22

@nreese

This comment has been minimized.

Contributor

nreese commented Nov 28, 2017

I could not get relative URLs like /app/kibana to work.

/vah/app/kibana works great.

Below is the Url Template

/vah/app/kibana#/discover?_g=(refreshInterval:(display:Off,pause:!f,value:0),time:(from:now-6M,mode:quick,to:now))&_a=(columns:!(_source),index:'8b473610-7ec0-11e7-b6a1-75e3bfc88121',interval:auto,query:(language:lucene,query:'geo.src:{{value}}'),sort:!('@timestamp',desc))

And the generated URL - which works

http://localhost:5601/vah/app/kibana#/discover?_g=(refreshInterval:(display:Off,pause:!f,value:0),time:(from:now-6M,mode:quick,to:now))&_a=(columns:!(_source),index:'8b473610-7ec0-11e7-b6a1-75e3bfc88121',interval:auto,query:(language:lucene,query:'geo.src:CN'),sort:!('@timestamp',desc))

/app/kibana does not work.

Below is the Url Template

/app/kibana#/discover?_g=(refreshInterval:(display:Off,pause:!f,value:0),time:(from:now-6M,mode:quick,to:now))&_a=(columns:!(_source),index:'8b473610-7ec0-11e7-b6a1-75e3bfc88121',interval:auto,query:(language:lucene,query:'geo.src:{{value}}'),sort:!('@timestamp',desc))

And the generated URL - which is broken. Notice the beginning http://localhost:5601/vah/app//app/kibana

http://localhost:5601/vah/app//app/kibana#/discover?_g=(refreshInterval:(display:Off,pause:!f,value:0),time:(from:now-6M,mode:quick,to:now))&_a=(columns:!(_source),index:'8b473610-7ec0-11e7-b6a1-75e3bfc88121',interval:auto,query:(language:lucene,query:'geo.src:CN'),sort:!('@timestamp',desc))

@chrisronline

This comment has been minimized.

Contributor

chrisronline commented Nov 28, 2017

@nreese Should /app/kibana work if a basePath is set? If you remove these changes and use a relative url with /app/kibana and a basePath set, it doesn't work either. I'm not sure we should worry about supporting that.

Thoughts?

@nreese

This comment has been minimized.

Contributor

nreese commented Nov 28, 2017

@chrisronline How would you support a relative link to another application in kibana without supporting /app/<app name>?

@chrisronline

This comment has been minimized.

Contributor

chrisronline commented Nov 28, 2017

@nreese You'd have to include the base path, right? /vah/app/<app name>? I think the basePath is just for development so production use cases should be able to use /app/<app name> and it will work fine. This code handles both scenarios, but please let me know if you don't think this is sufficient.

Are you thinking that we should support /app/<app name> even if there is a configured base path?

@nreese

nreese approved these changes Nov 29, 2017

lgtm

@chrisronline chrisronline merged commit d06ee13 into elastic:master Nov 29, 2017

2 checks passed

CLA Commit author is a member of Elasticsearch
Details
kibana-ci Build finished.
Details

chrisronline added a commit to chrisronline/kibana that referenced this pull request Nov 29, 2017

Prepend relative urls (elastic#14994)
* Add url whitelist

* Use .some

* Add/fix tests

* Add browser tests

* Initial pass

* Different approach, and more tests

* Fix linting issues

* Special case relative urls starting with `#`

* Handle root relative link too

* Try this another way

* Reapply my changes

chrisronline added a commit to chrisronline/kibana that referenced this pull request Nov 29, 2017

Prepend relative urls (elastic#14994)
* Add url whitelist

* Use .some

* Add/fix tests

* Add browser tests

* Initial pass

* Different approach, and more tests

* Fix linting issues

* Special case relative urls starting with `#`

* Handle root relative link too

* Try this another way

* Reapply my changes

@chrisronline chrisronline deleted the chrisronline:enhancement/relative-url branch Nov 29, 2017

chrisronline added a commit to chrisronline/kibana that referenced this pull request Nov 29, 2017

chrisronline added a commit to chrisronline/kibana that referenced this pull request Nov 29, 2017

chrisronline added a commit that referenced this pull request Nov 29, 2017

Prepend relative urls (#14994) (#15249)
* Add url whitelist

* Use .some

* Add/fix tests

* Add browser tests

* Initial pass

* Different approach, and more tests

* Fix linting issues

* Special case relative urls starting with `#`

* Handle root relative link too

* Try this another way

* Reapply my changes

chrisronline added a commit that referenced this pull request Nov 29, 2017

Prepend relative urls (#14994) (#15250)
* Add url whitelist

* Use .some

* Add/fix tests

* Add browser tests

* Initial pass

* Different approach, and more tests

* Fix linting issues

* Special case relative urls starting with `#`

* Handle root relative link too

* Try this another way

* Reapply my changes

chrisronline added a commit that referenced this pull request Nov 29, 2017

chrisronline added a commit that referenced this pull request Nov 29, 2017

@chrisronline

This comment has been minimized.

Contributor

chrisronline commented Nov 29, 2017

Backport

6.x: daab94b
6.1: 8e419e7
6.0: 4df8a0b
5.6: fdd62b0

chrisronline added a commit to chrisronline/kibana that referenced this pull request Dec 1, 2017

Prepend relative urls (elastic#14994)
* Add url whitelist

* Use .some

* Add/fix tests

* Add browser tests

* Initial pass

* Different approach, and more tests

* Fix linting issues

* Special case relative urls starting with `#`

* Handle root relative link too

* Try this another way

* Reapply my changes

@epixa epixa referenced this pull request Jan 9, 2018

Merged

Migrating vega_vis from plugin #15014

15 of 15 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment